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

Do not list related field choices in OPTIONS requests. #4021

Merged

Conversation

charettes
Copy link
Contributor

@charettes charettes commented Mar 29, 2016

Listing related fields can leak sensitive data and result in poor performance
when dealing with large result sets.

Large result sets should be exposed by a dedicated endpoint instead.

@charettes charettes changed the title Fixed #3751 -- Stopped listing all related field choices through metadata. Fixed #3751 -- Stopped listing related field choices through metadata. Mar 29, 2016
…tadata.

Listing related fields can leak sensitive data and result in poor performance
when dealing with large result sets.

Large result sets should be exposed by a dedicated endpoint instead.
@charettes charettes force-pushed the related-field-choices-metadata branch from 69c69b8 to a6732e2 Compare Mar 29, 2016
@xordoquy
Copy link
Collaborator

xordoquy commented Mar 29, 2016

I'm tempted to move this through the deprecation path.
The alternative would be to consider this a security improvement and go through in which case I'd probably add an option to turn this off and keep compatibility.

@xordoquy xordoquy added this to the 3.4.0 Release milestone Mar 29, 2016
@charettes
Copy link
Contributor Author

charettes commented Mar 29, 2016

I have deprecation path in mind, I'll submit it in a few moment.

@xordoquy
Copy link
Collaborator

xordoquy commented Mar 29, 2016

Thanks !

@tomchristie
Copy link
Member

tomchristie commented Mar 30, 2016

I'd probably be okay with us simply dropping this in a median version, so long as we call it out.
We don't have any strict contract around what to expect from OPTIONS responses, so...

@craigds
Copy link
Contributor

craigds commented Apr 22, 2016

Could this be merged? This fixes #3751 which is a security (and major performance) issue so seems important to get it in.

@tomchristie tomchristie merged commit 014e24b into encode:master Jun 1, 2016
2 checks passed
@tomchristie tomchristie changed the title Fixed #3751 -- Stopped listing related field choices through metadata. Do not list related field choices in OPTIONS requests. Jun 1, 2016
@tomchristie
Copy link
Member

tomchristie commented Jun 1, 2016

Great stuff, thank you!

@charettes charettes deleted the related-field-choices-metadata branch Jun 1, 2016
@silviogutierrez
Copy link
Contributor

silviogutierrez commented Jun 8, 2016

Hey guys,

Fantastic library and great work overall. For those that actually do use this feature, will there be an opt-in workaround[1]? I looked at the merge commit and it seems like a blanket check for all related fields.

It's pretty convenient to build a form off a single OPTIONS request.

Thanks for all your work,

Silvio

[1]: Mandatory: https://xkcd.com/1172/

@tomchristie
Copy link
Member

tomchristie commented Jun 8, 2016

You'd need to use a custom metadata class, overriding get_field_info so that you have the 3.3.x behavior, not the 3.4.x behavior. We should include that in the release notes.

@wimglenn
Copy link
Contributor

wimglenn commented Sep 12, 2016

Related example of overriding get_field_info: http://stackoverflow.com/q/35564784/674039

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