Opened at 2012-09-14T21:02:42Z
Closed at 2013-07-18T19:51:35Z
#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)
Change History (7)
Changed at 2012-09-15T03:38:24Z by davidsarah
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: ↓ 3 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 #942 is implemented 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.
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:5 Changed at 2013-06-16T22:27:21Z by daira
Pull request at https://github.com/tahoe-lafs/tahoe-lafs/pull/49.
comment:6 Changed at 2013-07-18T19:51:35Z by Daira Hopwood <daira@…>
- Resolution set to fixed
- Status changed from new to closed
src/allmydata/uri.py: Remove unused 'human encoding' methods, and associated tests. fixes #1807