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

Stream Collector: Cookie Bounce feature would not work if the tracker sends the payload via POST #27

Open
jankoulaga opened this issue Nov 16, 2017 · 4 comments

Comments

@jankoulaga
Copy link
Contributor

There are a few issues here.

  • The cookie bounce will never happen for POST, because in that case, response is not expected to be a pixel, so this branch
    val bounce = config.cookieBounce.enabled && nuidOpt.isEmpty && !bouncing && pixelExpected && !redirect
    will never be true.
  • In case of post, the returning status code must be 307, otherwise the client will not re-use the same payload for a redirect, and that's where the bigger problem lies. Some browsers (like Firefox) will not follow a 307 POST redirect unless the xhr content type is set to application/x-www-form-urlencoded.

So let's assume that i've modified the collector to return a 307, i've modified the JS tracker to send data as urlencoded form, the next thing that needs to be done is to allow that content type to be pushed to the sink.

So far it looks like a lot of things need to be changed, and still there are no guarantees that it would work for esoteric browsers such as IE 8.

Do you guys have any thoughts on this problem and the right way to solve it?

@BenFradet
Copy link
Contributor

It's true that cookie bounce was meant to support get requests only.

Your approach looks good to me. However, I have trouble understanding what the problem would be with

the next thing that needs to be done is to allow that content type to be pushed to the sink

? It seems to me like it's just another route in the collector?

@jankoulaga
Copy link
Contributor Author

jankoulaga commented Nov 17, 2017

@BenFradet Maybe then the release docs should be updated, saying that this feature only works for GET requests.

Now, about the ContentType... I was testing my hack on 0-8-0 with cookie bounce(that's the version we used to submit the Pull Request for the cookie bounce), and my Content-Type application/x-www-form-urlencoded would pass fine through the collector, but then i guess during enrichment it would end up in the bad stream due to:

{
  "level": "error",
  "message": "Content type of application/x-www-form-urlencoded provided, expected one of: application/json, application/json; charset=utf-8, application/json; charset=UTF-8"
}

What i'm seeing right now in the enricher, is that it actually could work with the latest release.

I'll try to whip up some pull requests for a POST cookie bounce(collector & JS tracker), if you guys are interested...

@BenFradet
Copy link
Contributor

A PR would be interesting indeed.

To come back to your earlier post, do you have references for:

Some browsers (like Firefox) will not follow a 307 POST redirect unless the xhr content type is set to application/x-www-form-urlencoded

?

@jankoulaga
Copy link
Contributor Author

Alright, i'll try to push something in the following week.

Unfortunately, my findings on POST redirects are purely empirical. What i did was a JS fiddle sending a xhr POST to a collector, just so i can prove that it was FF that's not behaving as it shuold.

First thing i tried was exactly the way how Snowplow JS tracker does it(with Json content type):

rq = new XMLHttpRequest();    
//THIS IS EXACTLY HOW SNOWPLOW DOES IT
rq.open("POST", redirectingURI, true);
rq.withCredentials = true;
rq.setRequestHeader('Content-Type', 'application/json; charset=utf-8');
rq.send(stringifiedJson);

The result of that was that in Chrome, the redirect was followed, data was submitted with the n3pc redirect; in FF, the redirect was not followed and the xhr threw an error; In Safari, the redirect was followed, however, the n3pc redirect contained no body.

Then by accident i commented out the content type setting, and the redirect worked in FF. The same request(without the content type) again didn't work in Safari, as the body was not added to the redirect request.

And then, after some reading about why 307 was introduced(one of the reason was posting form redirects), i tried using application/x-www-form-urlencoded and it worked.

@lukeindykiewicz lukeindykiewicz transferred this issue from snowplow/snowplow Jun 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants