Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-35838: Fix duplicate optionxform calls on a section's option #11760

Closed
wants to merge 3 commits into from

Conversation

tirkarthi
Copy link
Member

@tirkarthi tirkarthi commented Feb 5, 2019

optionxform is called to check for duplicates and the transformed key is passed to set which again calls optionxform making two transformations to the key. This can be a problem when optionxform is not idempotent.

https://bugs.python.org/issue35838

@tirkarthi
Copy link
Member Author

Friendly ping @methane . It would be helpful if this can be reviewed, I think it's an issue or this can be documented as in https://bugs.python.org/issue35838#msg334452

@methane
Copy link
Member

methane commented Mar 6, 2019

Did you confirm non-idempotent optionxform is supported all other cases?

I fixed the issue locally by way similar to you. But I was not sure about it.
So I haven't sent a PR.

@tirkarthi
Copy link
Member Author

From my reading of the code I couldn't come across any place where the optionxform is called twice causing a problem with respect to being idempotent. My initial guess was about template interpolation but they also apply optionxform only once and using optionxform = lambda x: f'{(x)}' didn't cause problem. There were also no docs or git log information where it was mentioned that optionxform should be idempotent. So I assumed it to be a bug. Sorry, I was not aware about the PR. Maybe @ambv might be able to have additional info about this.

@methane
Copy link
Member

methane commented Mar 6, 2019

I couldn't come across any place where the optionxform is called twice

Even if all APIs doesn't call optionxform twice, it can be called twice while using some APIs.
For example:

>>> import configparser
>>> cfg = configparser.ConfigParser()
>>> cfg.optionxform = lambda s: "#"+s
>>> cfg.add_section("sec")
>>> cfg.set("sec", "foo", "1")
>>> cfg["sec2"] = cfg["sec"]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/inada-n/work/python/cpython/Lib/configparser.py", line 974, in __setitem__
    self.read_dict({key: value})
  File "/Users/inada-n/work/python/cpython/Lib/configparser.py", line 747, in read_dict
    for key, value in keys.items():
  File "/Users/inada-n/work/python/cpython/Lib/_collections_abc.py", line 744, in __iter__
    yield (key, self._mapping[key])
  File "/Users/inada-n/work/python/cpython/Lib/configparser.py", line 1254, in __getitem__
    raise KeyError(key)
KeyError: '#foo'

(Note that I checked out this PR and tested above case)

@tirkarthi
Copy link
Member Author

Closing this PR as there is no consensus on optionxform to be idempotent. I will reopen it if needed. Thanks @methane for the details.

@tirkarthi tirkarthi closed this Apr 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants