#1807 closed defect (fixed)

cleanup: HUMAN_RE regexes in uri.py are never used

Reported by: davidsarah Owned by: daira
Priority: minor Milestone: 1.10.1
Component: code Version: 1.9.2
Keywords: cleanup url reviewed Cc:
Launchpad Bug:

Description (last modified by zooko)

From src/allmydata/uri.py:

# "human-encoded" URIs are allowed to come with a leading
# 'http://127.0.0.1:(8123|3456)/uri/' that will be ignored.
# Note that nothing in the Tahoe code currently uses the human encoding.

We should remove the dead code related to "human encoding" and instead make the normal parsing of URIs more tolerant: see #942, #884, and #885.

Attachments (1)

fix-1807.darcs.patch (152.2 KB) - added by davidsarah at 2012-09-15T03:38:24Z.
src/allmydata/uri.py: Remove unused 'human encoding' methods, and associated tests. fixes #1807

Download all attachments as: .zip

Change History (7)

Changed at 2012-09-15T03:38:24Z by davidsarah

src/allmydata/uri.py: Remove unused 'human encoding' methods, and associated tests. fixes #1807

comment:1 Changed at 2012-09-15T03:38:56Z by davidsarah

  • Keywords review-needed added
  • Owner changed from davidsarah to warner

comment:2 follow-up: Changed at 2012-09-15T03:44:25Z by zooko

Why not use the "human encoding" parser of URLs instead of removing it and making the normal parsing of URLs more like it?

comment:3 in reply to: ↑ 2 Changed at 2012-09-15T18:54:38Z by davidsarah

Replying to zooko:

Why not use the "human encoding" parser of URLs instead of removing it and making the normal parsing of URLs more like it?

Because:

  • there's no ticket proposing to allow an http://... prefix for Tahoe URIs, and I don't think we want that.
  • the human encoding parsers don't implement #884 or #885. They partially implement #942, but inconsistently, for example most of the ":" characters can be replaced by "%3A" (matched case-sensitively, which is wrong), but the one before an MDMF extension field cannot. This is because the human encoding implementation approaches this in a suboptimal way: we should be %-unencoding the whole Tahoe URI rather than treating ":" as a special case.
  • making this change has very little regression risk because it is only dead code that is being removed.
  • most of the work of implementing #942, #884, and #885 is in adding and repairing tests, the non-test code changes are straightforward. So not much effort is actually saved by reusing the human encoding code.
Last edited at 2012-09-15T19:02:41Z by davidsarah (previous) (diff)

comment:4 Changed at 2013-06-07T00:01:30Z by zooko

  • Description modified (diff)
  • Keywords reviewed added; review-needed removed
  • Owner changed from warner to daira

Okay, I agree! I scanned this patch and it looks good because it is all code-removal.

comment:6 Changed at 2013-07-18T19:51:35Z by Daira Hopwood <daira@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 2bbfc7927d8a3c2e58a9ca2907763bb13d3be70d/trunk:

src/allmydata/uri.py: Remove unused 'human encoding' methods, and associated tests. fixes #1807

Signed-off-by: Daira Hopwood <david-sarah@…>

Note: See TracTickets for help on using tickets.