cleanup: HUMAN_RE regexes in uri.py are never used #1807

New Issue

tahoe-lafs · 2012-09-14T21:02:42Z

davidsarah commented

2012-09-14 21:02:42 +00:00

From source: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.

From source: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.

tahoe-lafs added the

labels 2012-09-14 21:02:42 +00:00

tahoe-lafs added this to the 1.10.1 milestone 2012-09-14 21:02:42 +00:00

davidsarah commented

2012-09-15 03:38:24 +00:00

Attachment fix-1807.darcs.patch (155847 bytes) added

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

**Attachment** fix-1807.darcs.patch (155847 bytes) added src/allmydata/uri.py: Remove unused 'human encoding' methods, and associated tests. fixes #1807

fix-1807.darcs.patch

152 KiB

zooko commented

2012-09-15 03:44:25 +00:00

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

davidsarah commented

2012-09-15 18:54:38 +00:00

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.

Replying to [zooko](/tahoe-lafs/trac-2024-07-25/issues/1807#issuecomment-89523): > 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.

zooko commented

2013-06-07 00:01:30 +00:00

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

daira commented

2013-06-16 22:27:21 +00:00

Pull request at https://github.com/tahoe-lafs/tahoe-lafs/pull/49.

Pull request at <https://github.com/tahoe-lafs/tahoe-lafs/pull/49>.

Daira Hopwood <daira@jacaranda.org> commented

2013-07-18 19:51:35 +00:00

In /tahoe-lafs/trac-2024-07-25/commit/2bbfc7927d8a3c2e58a9ca2907763bb13d3be70d:

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

Signed-off-by: Daira Hopwood <david-sarah@jacaranda.org>

In [/tahoe-lafs/trac-2024-07-25/commit/2bbfc7927d8a3c2e58a9ca2907763bb13d3be70d](/tahoe-lafs/trac-2024-07-25/commit/2bbfc7927d8a3c2e58a9ca2907763bb13d3be70d): ``` src/allmydata/uri.py: Remove unused 'human encoding' methods, and associated tests. fixes #1807 Signed-off-by: Daira Hopwood <david-sarah@jacaranda.org> ```

tahoe-lafs added the

fixed

label 2013-07-18 19:51:35 +00:00

Daira Hopwood <daira@jacaranda.org> closed this issue

2013-07-18 19:51:35 +00:00

Sign in to join this conversation.