Opened 9 years ago
Closed 2 years ago
#11538 closed defect (fixed)
Duplicate options are allowed in sections of trac.ini
|Reported by:||Ryan J Ollos||Owned by:||Ryan J Ollos|
Description (last modified by )
The sections of trac.ini allow duplicate options, with later values overwriting those that come first. Two patterns that could lead to confusing behavior when the user doesn't realize that the option's value is being changed later in the file are:
- Duplicate sections with the same option.
[trac] default_handler = WikiModule ... [trac] default_handler = TicketModule
- Duplicate options within a section.
[trac] default_handler = WikiModule ... default_handler = TicketModule
While Python 3.x has a much improved configparser module with a
strict mode that raises an error when an option appears more than once, I'm not sure there is much we can do in Python 2.x using
SO:5396653 suggests using a
dict_type that allows multiple values with the same key, however
dict_type is only supported in Python 2.6, so we wouldn't be able to apply as
dict_type-based fix to 1.0-stable.
Change History (11)
comment:1 by , 9 years ago
comment:2 by , 8 years ago
comment:3 by , 3 years ago
|Milestone:||undecided → 1.5.2|
comment:4 by , 3 years ago
comment:5 by , 3 years ago
|Status:||new → assigned|
comment:6 by , 2 years ago
|Milestone:||1.5.2 → 1.5.3|
I think this could avoid some difficult to find problems. Examples:
Internal Server Error TracError: DuplicateSectionError: While reading from '/Users/rjollos/Documents/Workspace/trac-dev/tracenvs/proj-1.5/conf/trac.ini' [line 8]: section 'attachment' already exists
Internal Server Error TracError: DuplicateOptionError: While reading from '/Users/rjollos/Documents/Workspace/trac-dev/tracenvs/proj-1.5/conf/trac.ini' [line 39]: option 'acct_mgr.register.*' in section 'components' already exists
Proposed changes in [00a4344992/rjollos.git]. Deferring to 1.5.3. Please let me know if you see any major problems with switching to strict parsing.
comment:7 by , 2 years ago
AuthzPolicy.parse_authz() should catch all
Errors of configparser when parsing the file.
self.authz = UnicodeConfigParser(ignorecase_option=False) try: self.authz.read(self.authz_file) - except ParsingError as e: + except configparser.Error as e: self.log.error("Error parsing authz permission policy file: %s", exception_to_unicode(e)) - raise ConfigurationError() + raise ConfigurationError(_("Error parsing authz permission policy " + "file: %(err)s", + err=exception_to_unicode(e)))
comment:8 by , 2 years ago
Errors is a good idea. Thanks.
When working on this module previously I intentionally didn't raise a message for
ConfigurationError because it seemed like a possible security risk.
comment:9 by , 2 years ago
Also noticed #13335, which I'll fix on 1.4-stable.
comment:10 by , 2 years ago
|Release Notes:||modified (diff)|
Committed to trunk in r17491.
comment:11 by , 2 years ago
|Status:||assigned → closed|
After #11982, which replaced use of
ConfigParser, an exception is no longer raised when there is a duplicate section or option in an
authz_policyconfiguration file. One test is commented out: trunk/tracopt/perm/tests/authz_policy.py@13994:223-234#L223.