Skip to content

Fix ident format when using HTTP_X_FORWARDED_FOR #2401

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

Merged
merged 2 commits into from
Jan 12, 2015
Merged

Fix ident format when using HTTP_X_FORWARDED_FOR #2401

merged 2 commits into from
Jan 12, 2015

Conversation

jpadilla
Copy link
Member

If NUM_PROXIES setting is set to None, HTTP_X_FORWARDED_FOR might be used as is, which
might contain spaces and cause errors on cache backends like memcached. Ref #2400

If NUM_PROXIES setting is set to None,
HTTP_X_FORWARDED_FOR might be used as is, which
might contain spaces and cause errors on
cache backends like memcached.
ident = request.META.get('REMOTE_ADDR')
else:
ident = ''.join(ident.split())

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see why those were removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

@xordoquy they weren't being used at all.

@jpadilla
Copy link
Member Author

Note: This happens in both v2 and v3.

@tomchristie
Copy link
Member

Looks okay to me - anyone else want to second it?

@@ -35,7 +35,7 @@ def get_ident(self, request):
client_addr = addrs[-min(num_proxies, len(xff))]
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned in #2400 (comment), this should be len(addrs) instead of len(xff).

@jpadilla jpadilla added this to the 3.0.4 Release milestone Jan 12, 2015
@jpadilla
Copy link
Member Author

This should be good to go. Thanks for the reviews.

tomchristie added a commit that referenced this pull request Jan 12, 2015
Fix ident format when using HTTP_X_FORWARDED_FOR
@tomchristie tomchristie merged commit 69fea56 into encode:master Jan 12, 2015
@jpadilla
Copy link
Member Author

@tomchristie not sure if it's worth fixing this in 2.4 as well. Seems to be workable around easily. Let's see if it comes up again.

@xordoquy
Copy link
Collaborator

The general idea was to drop the 2.4 support since our time is limited.

@tomchristie
Copy link
Member

not sure if it's worth fixing this in 2.4 as well

I'd have no great argument if somebody wanted to spend their time on that, but I don't think it's a good effort/reward myself, so unless someone wants to get really proactive with making a further 2.4.x release happen then it's a no from me.

See https://groups.google.com/forum/#!searchin/django-rest-framework/2.4/django-rest-framework/NHcDGGPaqNw/Y64_7ipJcX0J

@jpadilla
Copy link
Member Author

👍

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.

4 participants