Skip to content

Fixed issue when user_agent exceeded 200 chars #14

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

Closed
wants to merge 1 commit into from
Closed

Fixed issue when user_agent exceeded 200 chars #14

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jul 30, 2014

No description provided.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.16%) when pulling 962419b on acatarineu:master into 105ec5c on Bouke:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.16%) when pulling 36eb0bf on acatarineu:master into 105ec5c on Bouke:master.

@Bouke
Copy link
Collaborator

Bouke commented Jul 31, 2014

@acatarineu thanks for your contribution. However I don't like adding a new field type, just for truncating to a particular length. I feel like there should be easier ways to do this. I'd be happy to merge an updated pull request that has: simpler implementation and a unit test that fails without the patch.

@Gwildor
Copy link

Gwildor commented Nov 4, 2014

I'm running into this issue as well. For example, I've seen users with the following user agent:

Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 6.1; WOW64; Trident/5.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C; .NET4.0E; InfoPath.3; ms-office; MSOffice 14)

Wouldn't it be easier and better to just use a text field?

@Bouke
Copy link
Collaborator

Bouke commented Nov 4, 2014

@acatarineu Where did you get that code snippet of TruncatingCharField, by any chance from this SO post? While I do promote code re-use, posting someone else's code without notice is certainly not ok. At least credit the author of the code and include a URL with the commit.

@Gwildor In my tests on SQLite this doesn't raise an issue. However it might give an issue with PostgreSQL, but I don't have access to that currently. Could you provide some more insight?

@Bouke Bouke closed this in 68c2961 Nov 4, 2014
@Bouke
Copy link
Collaborator

Bouke commented Nov 4, 2014

To resolve the issue with long user_agent strings, there are two options:

  1. Increase the length of the field. But how long should the field be? There doesn't appear a maximum on the length of the user agent, so theoretical the field should be as long as the headers permit. A few kB perhaps?
  2. Truncate the user agent. Microsoft recommends the user agent to be 200 characters maximum. Also, the most important information is near the front of the user agent (browser + OS and resp. versions).

This app doesn't require the full user agent anyway, so I chose the second option. I've pushed a commit that will truncate the user_agent to max of 200 characters. Could you confirm that this fixes the issue you're having?

@Gwildor
Copy link

Gwildor commented Nov 6, 2014

I can confirm that this issue has been fixed as well.

@Bouke
Copy link
Collaborator

Bouke commented Nov 11, 2014

Thanks :). I've also pushed a new version (1.1.0) containing this fix.

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.

3 participants