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

Prevent property with trailing equal sign in config set #3567

Merged
merged 2 commits into from Mar 5, 2015

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Mar 4, 2015

See https://www.openmicroscopy.org/community/viewtopic.php?f=5&t=7752#p15430

This PR should prevent an operation like

bin/omero config set omero.property= value

Apart from checking the unit tests are green in Travis, testing this PR should include running this command and testing config.xml remains unmodified.

--no-rebase

@sbesson sbesson changed the title CLI config set trailing equal Prevent property with trailing equal sign in config set Mar 4, 2015
@sbesson sbesson added the develop label Mar 4, 2015
@manics
Copy link
Member

manics commented Mar 5, 2015

Is there a specification for allowed characters in keys? E.g. if it's limited to [A-Za-z0-9\.]+ could you do a more thorough check?

@joshmoore
Copy link
Member

There's not really anything that's filtered. This is mostly a usability check in terms of "a=b" is probably not what you mean, since that means "remove the key named 'a=b'". If we added "bin/omero config unset a=b" and deprecate the one arg usage, perhaps this would get easier.

@manics
Copy link
Member

manics commented Mar 5, 2015

Good to merge then.

joshmoore added a commit that referenced this pull request Mar 5, 2015
Prevent property with trailing equal sign in config set
@joshmoore joshmoore merged commit 9b14aa1 into ome:develop Mar 5, 2015
@sbesson sbesson deleted the config_equal branch March 5, 2015 11:05
@sbesson sbesson added this to the 5.1.0-m5 milestone Mar 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants