Opened 8 years ago

Closed 8 years ago

# CustomFieldAdminPlugin should validate input parameters

Reported by: Owned by: Emmanuel Blot Odd Simon Simonsen highest CustomFieldAdminPlugin normal 0.11

### Description

The plugin does not check for non-ASCII characters in the custom field values ("select box", at least)

It then updates the Trac configuration values with unsupported character and asks Trac to save the new configuration to the configuration file.

Trac does not handle such characters, and destroy the configuration file as an exception is raised while overwriting the existing configuration file.

I'll update this ticket with the trac related ticket when/if trac.edgewall.org comes back to life.

### comment:2 follow-up:  3 Changed 8 years ago by Odd Simon Simonsen

Well, actually it does verify - the plugin ensures that all items can be encoded to UTF-8. This was done as [5161] from similar report in #4524.

You are of course running latest 0.11 version of the plugin?

### comment:3 in reply to:  2 Changed 8 years ago by Emmanuel Blot

You are of course running latest 0.11 version of the plugin?

I'm running TracCustomFieldAdmin-0.2-py2.5.egg, which seems to match (0.2) the version in the setup.py file.

### comment:4 Changed 8 years ago by Emmanuel Blot

Wow, the setup.py file has not been updated for 2 years, while the plugin code is 7 month old, so it's hard to tell which version I actually run.

### comment:5 Changed 8 years ago by Odd Simon Simonsen

I've bumped the version for you - and version should now also appear in 'About Trac'. See [6469].

Could you try verifying your version again?

### comment:6 follow-up:  7 Changed 8 years ago by Christian Boos

[5161] produces UTF-8 encoded str, and this seems to be fed into the Config API which expects unicode objects (as nearly any other Trac API). This is quite wrong.

### comment:7 in reply to:  6 ; follow-ups:  9  10 Changed 8 years ago by Odd Simon Simonsen

[5161] produces UTF-8 encoded str, and this seems to be fed into the Config API which expects unicode objects (as nearly any other Trac API). This is quite wrong.

Well, that is a matter for discussion as configuration has until a few changesets ago (in 0.11.6dev) not handled unicode well at all. Passing unicode would result in the mentioned error when config would write them to disk - if the unicode contained anything but plain ascii. I pass in utf-8, which gets saved, causing config to reload with unicode values. Remember custom fields also affects the options in the configs (keys), and not just their values.

### comment:8 Changed 8 years ago by Odd Simon Simonsen

Actually, I have other safety mechanisms in place as well to ensure that custom field names does not contain non-ascii characters... Hmm... Don't remember all the details anymore :-)

### comment:9 in reply to:  7 ; follow-up:  11 Changed 8 years ago by Emmanuel Blot

Well, that is a matter for discussion as configuration has until a few changesets ago (in 0.11.6dev) not handled unicode well at all. Passing unicode would result in the mentioned error when config would write them to disk - if the unicode contained anything but plain ascii. I pass in utf-8, which gets saved, causing config to reload with unicode values. Remember custom fields also affects the options in the configs (keys), and not just their values.

AFAIR, it was decided a long time ago that all internal strings in Trac should be unicode only.

IMHO, the Configuration module should load and store trac.ini file with UTF-8 encoding. I think there is an old ticket related with this topic: how to specify the encoding of trac.ini file, I can't remember which one.

### comment:10 in reply to:  7 ; follow-up:  14 Changed 8 years ago by Christian Boos

Remember custom fields also affects the options in the configs (keys), and not just their values.

Ah yes, that's indeed a problem. But the fact that converting both keys and values to utf-8 just before saving the configuration has worked at all was just luck.

I understand that changing back to unicode wouldn't work with earlier 0.11 versions, but neither will the current code work with 0.11.6. That's a bit unfortunate, we should maybe have fixed #8162 for trunk only. Would it be possible to check for the Trac version?

### comment:11 in reply to:  9 ; follow-up:  12 Changed 8 years ago by Christian Boos

AFAIR, it was decided a long time ago that all internal strings in Trac should be unicode only.

... which doesn't mean that unicode related bugs still surface from time to time. #T8162 was one of them.

IMHO, the Configuration module should load and store trac.ini file with UTF-8 encoding.

I think there is an old ticket related with this topic: how to specify the encoding of trac.ini file, I can't remember which one.

I don't remember having seen this.

### comment:12 in reply to:  11 ; follow-up:  13 Changed 8 years ago by Emmanuel Blot

#T8162 was one of them.

IMHO, the Configuration module should load and store trac.ini file with UTF-8 encoding.

Ok, I got confused.

I don't remember having seen this.

I'm unable to find the reference either, but I'm pretty sure it was left as an open question ;-)

### comment:13 in reply to:  12 Changed 8 years ago by Christian Boos

#T8162 was one of them.

Bad day for ticket references today ... I meant #T8276.

### comment:14 in reply to:  10 ; follow-ups:  15  16 Changed 8 years ago by Odd Simon Simonsen

I understand that changing back to unicode wouldn't work with earlier 0.11 versions, but neither will the current code work with 0.11.6. That's a bit unfortunate, we should maybe have fixed #8162 for trunk only. Would it be possible to check for the Trac version?

Not quite true - I'm running latest plugin (unchanged for a long time) with very latest 0.11.6dev. Ascii or non-ascii for values, and nothing like eblots problem is seen at my end. All works fine for me.

eblot, you are on latest plugin version now right? And you can reproduce the problem? Remember to backup configuration file... :-)

### comment:15 in reply to:  14 Changed 8 years ago by Emmanuel Blot

eblot, you are on latest plugin version now right? And you can reproduce the problem? Remember to backup configuration file... :-)

Very, very busy right now, will try soon. Hopefully, my configuration files are backed up once a day ;-)

### comment:16 in reply to:  14 Changed 8 years ago by anonymous

I understand that changing back to unicode wouldn't work with earlier 0.11 versions, but neither will the current code work with 0.11.6. That's a bit unfortunate, we should maybe have fixed #8162 for trunk only. Would it be possible to check for the Trac version?

Not quite true - I'm running latest plugin (unchanged for a long time) with very latest 0.11.6dev. Ascii or non-ascii for values, and nothing like eblots problem is seen at my end. All works fine for me.

Ok, that's probably because there are a bunch of to_unicode calls in various places in config.py ;-) Still, maybe the 0.12 version of the plugin could try to use unicode only, then.

### comment:17 Changed 8 years ago by Emmanuel Blot

eblot, you are on latest plugin version now right? And you can reproduce the problem? Remember to backup configuration file... :-)

Ok, I've updated to the 0.2.2 version, and I cannot reproduce this issue anymore. Thanks for your help.

### comment:18 Changed 8 years ago by Emmanuel Blot

Resolution: → worksforme new → closed

### Modify Ticket

Change Properties