comments in tahoe.cfg cause introducers to appear down

[amiller]

This causes introducer to appear disconnected in webview

[client] introducer.furl = pb://censored@censored/introducer # thanks zancas!

This works fine

[client] introducer.furl = pb://censored@censored/introducer

[zooko]

related or possible duplicate issues: #371, #649

[amiller]

So this seems to be due to the file format of ConfigParser?.

​http://docs.python.org/2/library/configparser.html (For backwards compatibility, only ; starts an inline comment, while # does not.)

I think a solution would be to simply throw an error if the furl contains whitespace, since furls shouldn't contain whitespace.

[zooko]

Shouldn't we also raise an exception if the furl contains a "#" ?

[amiller]

Added a test that illustrates this: ​https://github.com/amiller/tahoe-lafs/commit/647825835700ceefea5e7018a805c250ae33178c

[daira]

I don't think we should treat furls as a special case, since the same issue occurs for any line of the config file that has # after a field entry, if I understand correctly. We could preprocess the file to either treat such comments as an error, or remove them.

[zooko]

I propose that we modify our "config getter/setter" code​ to raise an exception any time a field name or value has a "#" character in it, both during read and during write.

[daira]

Replying to zooko:

I propose that we modify our "config getter/setter" code​ to raise an exception any time a field name or value has a "#" character in it, both during read and during write.

That seems reasonable. Are we sure that no field values need to contain #s? (I'm particularly thinking of fields that include syntax we haven't defined, like Twisted endpoint strings.)

[zooko]

Replying to daira:

Replying to zooko:

I propose that we modify our "config getter/setter" code​ to raise an exception any time a field name or value has a "#" character in it, both during read and during write.

That seems reasonable. Are we sure that no field values need to contain #s? (I'm particularly thinking of fields that include syntax we haven't defined, like Twisted endpoint strings.)

I'm not sure! What should we do?

[daira]

Replying to zooko:

Replying to daira:

Replying to zooko: [...] Are we sure that no field values need to contain #s? (I'm particularly thinking of fields that include syntax we haven't defined, like Twisted endpoint strings.)

I'm not sure! What should we do?

I guess we first read docs/configuration.rst​ looking for fields that might contain #s.

[daira]

Endpoint strings use \-escaping, so if we allow \#, then we're not preventing any endpoint from being expressed.

[amiller]

I made a quick fix (that passes my test), that raise an exception whenever the introducer furl contains a #

​https://github.com/amiller/tahoe-lafs/commit/73eeb826bd986245e105abadc0ab50550e94e06f

[amiller]

Replying to amiller:

I made a quick fix (that passes my test), that raise an exception whenever the introducer furl contains a #

​https://github.com/amiller/tahoe-lafs/commit/73eeb826bd986245e105abadc0ab50550e94e06f

Er, I just realized I missed the entire point of comment 12, will fix

[amiller]

I now have a regular expression that tests for an unescaped # in the furl. However I don't think it's complete. The regexp is ([\^\\\\]|^)# which matches any # that is not preceded by a backslash.

My test includes the following:

        assert re.search(regexp, "test#test") is not None
        assert re.search(regexp, "#testtest") is not None
        assert re.search(regexp, "test\#test") is None
        assert re.search(regexp, "testtest") is None

Unfortunately I think the string test\\#test should probably be matched, but it isn't. I'd need to look at how the \-escaping works to be sure.

​https://github.com/amiller/tahoe-lafs/commit/47694c89d0e40ca869f74a53fee9703349dca474

[daira]

I continue to think we should not treat furls as a special case (comment:7).

[zooko]

Replying to daira:

I continue to think we should not treat furls as a special case (comment:7).

So what if ​Andrew's patch were applied to all values from the config. Would that be better?

Also, daira, what should we do about a string like "test\\#test" in a value from the config?

[daira]

Replying to zooko:

Replying to daira:

I continue to think we should not treat furls as a special case (comment:7).

So what if ​Andrew's patch were applied to all values from the config. Would that be better?

Yes, the approach in comment:8 seems like a reasonable way of doing that. (It only checks values that are used, rather than all lines of the file, but that should be fine.)

Also, daira, what should we do about a string like "test\\#test" in a value from the config?

That string doesn't contain an escaped # (only an escaped \), so it contains a #-comment and should be rejected. I don't understand the regexp in comment:15 though. (BTW, I suggest using raw r"..." strings for regexps, to avoid the extra level of \-escaping. Triple-quoted strings aren't necessary.)

[amiller]

I was distracted previously by the config parser, but it's actually the ​twisted endpoint parser that matters. My approach ​in this latest patch is to emulate this function except for rejecting unescaped # - the case of r"test\\#test" is correctly rejected, for example.

Now to address comment:8, about applying this check uniformly to every endpoint string in the config... it appears that every furl in the config has a label of the form "blah.furl". Should I rely on this and put the check in node.get_config?

[amiller]

I took the test that makes sure furl endpoint strings don't contain unescaped hashes, and added it to node.get_config where it will be applied uniformly to every config entry of the form something.furl. I added a new test to guarantee that it's still okay to have a hash in a nickname, for example "Hash#Bang!" is still OK to use. ​https://github.com/amiller/tahoe-lafs/commit/64ddd08354822d9789f15ad57b4b769ea74b328f

[Daira Hopwood <daira@…>]

In 883dd9795bcf4f309222f4f62038537c24d0791f/trunk:

Error if a .furl config entry contains an unescaped '#'. fixes #2128

Author: Andrew Miller <amiller@…>
Signed-off-by: Daira Hopwood <daira@…>

[daira]

... and [87df9d218d9b6435dbb0f93925be91a855d700e8/trunk], which was supposed to be in the same changeset but I forgot to commit it.

[Daira Hopwood <daira@…>]

In efc223ad1fdefd39827b5516a37b7b5822532bbc/trunk:

Improve error reporting for '#' characters in config entries. refs #2128

Signed-off-by: Daira Hopwood <daira@…>