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

Fix 500 error when attempting to update installed repository. #1082

Merged
merged 3 commits into from Nov 14, 2015

Conversation

davebx
Copy link
Contributor

@davebx davebx commented Nov 13, 2015

No description provided.

@@ -335,6 +335,17 @@ def shrink_string_by_size( value, size, join_by="..", left_larger=True, beginnin
return value


def parse_query_string( query_string ):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this function be able to deal with malicious query_strings ? E.g.: 'foo=3&bar=4&', 'foo&pluto=4'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it will be necessary, but there's absolutely no harm in preventing malice.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brilliant, thanks! I think that now that you've added if '=' not in argument, you can remove:

    if '=' not in query_string:
        return {}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@carlfeberhard reminded me that urlparse.parse_qs exists.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!

@martenson
Copy link
Member

is there an example of repo that has this problem? @bgruening @davebx

@davebx
Copy link
Contributor Author

davebx commented Nov 13, 2015

@martenson I've been able to reliably cause the error locally, but not when installing or updating from TTS.

@yhoogstrate
Copy link
Member

This PR solves the issue for me, thanks @davebx !

martenson added a commit that referenced this pull request Nov 14, 2015
Fix 500 error when attempting to update installed repository.
@martenson martenson merged commit 3ba139f into galaxyproject:dev Nov 14, 2015
@bgruening
Copy link
Member

@davebx @martenson this is a bugfix can we get it into 15.10?

@martenson
Copy link
Member

@bgruening I am working on that already, stay tuned! :)

@bgruening
Copy link
Member

Thanks!

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.

None yet

6 participants