Skip to content

Fixed #2250#2271

Merged
sigmavirus24 merged 3 commits intopsf:masterfrom
mikecool1000:fix-2250
Oct 10, 2014
Merged

Fixed #2250#2271
sigmavirus24 merged 3 commits intopsf:masterfrom
mikecool1000:fix-2250

Conversation

@mikecool1000
Copy link
Contributor

Fixed #2250 with #2271

Fixed -2250
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regex is more capable than string utils

Copy link
Contributor

Choose a reason for hiding this comment

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

Also much slower.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, please put a after the comma, e.g.,split(",\ *<", value).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

On Thu, Oct 9, 2014 at 8:07 AM, Ian Cordasco notifications@github.com
wrote:

In requests/utils.py:

@@ -567,7 +567,7 @@ def parse_header_links(value):

 replace_chars = " '\""
  • for val in value.split(","):
  • for val in re.split(",\ *<",value):

Also, please put a after the comma, e.g., split(",\ *<", value).


Reply to this email directly or view it on GitHub
https://github.com/kennethreitz/requests/pull/2271/files#r18650977.

@mikecool1000 mikecool1000 changed the title Update utils.py Fixed #2250 Oct 9, 2014
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you have a \ in the expression? I'm very certain that's unnecessary if you're trying to say "zero or more s".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're completely right, sorry I do most of my regex in js, where it would be necessary sometimes

Copy link
Contributor

Choose a reason for hiding this comment

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

No worries. I should have caught that earlier.

sigmavirus24 added a commit that referenced this pull request Oct 10, 2014
@sigmavirus24 sigmavirus24 merged commit 2ac771b into psf:master Oct 10, 2014
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Link header parsing breaks if attribute value has commas

2 participants