Skip to content
This repository has been archived by the owner on Oct 23, 2023. It is now read-only.

Modify http data string sanitizing to account for cookie delimiter #535

Merged
merged 1 commit into from Feb 11, 2015

Conversation

jbarbuto
Copy link
Contributor

@jbarbuto jbarbuto commented Dec 6, 2014

When sanitizing http data strings, a & was treated as the only possible delimiter, while cookies use ; instead. This accounts for the cookie delimiter as well.

This assumes that a string is the proper type for cookie data in the http interface, as sent by the tornado client in https://github.com/getsentry/raven-python/blob/5.1.1/raven/contrib/tornado/__init__.py#L191 and shown in http://sentry.readthedocs.org/en/latest/developer/interfaces/#sentry.interfaces.http.Http. The previous tests indicate it's a dict instead.

@jbarbuto jbarbuto force-pushed the sanitize_http_cookies branch 3 times, most recently from 499181f to 1e2ea5c Compare December 6, 2014 21:06
@jbarbuto
Copy link
Contributor Author

jbarbuto commented Dec 6, 2014

A closer look at the code showed cookies can be either a dict or string, so the tests now handle both.

@xordoquy
Copy link
Contributor

xordoquy commented Dec 6, 2014

Great ! I need to dig a bit this part of raven but looks rather good at first sight.

@adamsc64
Copy link

👍

@xordoquy
Copy link
Contributor

@jbarbuto grats for the work and thanks a lot. Sorry it took me so long to review.

xordoquy added a commit that referenced this pull request Feb 11, 2015
Modify http data string sanitizing to account for cookie delimiter
@xordoquy xordoquy merged commit 1e893c9 into getsentry:master Feb 11, 2015
@jbarbuto jbarbuto deleted the sanitize_http_cookies branch February 24, 2015 05:43
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 8a12fa1 on jbarbuto:sanitize_http_cookies into * on getsentry:master*.

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.

None yet

4 participants