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

try to use referer when the path is not forced #41

Merged
merged 4 commits into from
Apr 9, 2016

Conversation

Guisardo
Copy link
Contributor

Hi there,
Thanks for sharing this project!
I think that this can be useful. If no path is passed and the referer header exists, use the referer as path.

FYI: I know that my code is not the pretiest one, but is working good enoght for me. Feel free to improve it.

Best,

@@ -103,16 +103,23 @@ func handler(w http.ResponseWriter, r *http.Request) {

// /account -> account template
if len(params) == 1 {
Copy link
Owner

Choose a reason for hiding this comment

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

This code is only invoked if you don't pass in the account name + path.. I'm not sure that's what you're after?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, when len(params) == 1 is because the path only contains the account. The account is still required, although we can probably set a default one in some configuration file if non is passed.

@igrigorik
Copy link
Owner

I'm not convinced this does what you think it does.. As in, you parse out Referer value, but that value is not used anywhere meaningful.

Also, while I understand the high-level convenience: initialize path from referrer if no path is given, in practice you can't count on the Refer being there due to cross-origin and cross-protocol (http -> https) fetches. Given that, we want to encourage folks to specify the path when recording a hit, and that hit path should override referer.. So, I'm not convinced that this is necessary at all? =/

@Guisardo
Copy link
Contributor Author

Hi @igrigorik , thanks for your comments.
Actually, the extra referrer parse I did it only to debug the code while I was trying it. I'm going to add the page.html version that I have too.

On the other hand, the referrer availability shouldn't be an issue, in fact, the referrer is commonly used on security algorithms for things like preventing hot linking. Although they are some odd browsers that don't always send the referrer, the amount of traffic coming from those browsers is usually not relevant at all. Of course that if you need to measure the traffic from those odd browsers you should use the forced path method, but I'm certain that less that 1% of the dev that could use this project, enters in this case scenario.
http://novogeek.com/post/What-web-devs-should-know-about-HTTP-Referer-header.aspx
http://homakov.blogspot.com.ar/2012/04/playing-with-referer-origin-disquscom.html

The cross protocol should't be an issue ether if you host this beacon inside ssl like, for example, the google cloud platform. The cross protocol raises when, from the https, you try to use http assets; not the other way around. So, if the beacon already have https, you can use it from http and https without any problems.

Then again, this implementation let you choose the method that you want you use to get the path, but I find a lot more simpler to trust the referrer instead of forcing the path in every place that I use the beacon. For example, I have a lot of ebay posts; is was virtually imposible to add the path in each and every one. With this, I only copy-pasted the beacon img.
Best,

@igrigorik
Copy link
Owner

FWIW, browsers do expose a way to control the referer policy: https://www.w3.org/TR/referrer-policy/

For example, I have a lot of ebay posts; is was virtually imposible to add the path in each and every one. With this, I only copy-pasted the beacon img.

Ah, ok, I see what you're after. However, I'm still not convinced the code is doing what you want it to do, because you're attempting to render an HTML page in response to an img request?

It seems like what you want is:

  • allow /{valid-GA-account}/hits
  • if no path is given, fallback to Referer
  • if path is present, use that override Referer

@Guisardo
Copy link
Contributor Author

Thanks for the comment @igrigorik .
You are correct, that flow is what I was looking for.

The HTML page was already in the code. It seems that works as some kind of error handling. I guess that we can return the page only when some "debug" parameter is sent; and if no debug paramente is sent but it fails, just return a status code of 500 or 404. What do you think?

I'm not sure on how to make this check

allow /{valid-GA-account}/hits

I'm not that skilled in golang to use regex or something like that. Can you help me with that code?
Best,

@igrigorik
Copy link
Owner

The HTML page was already in the code. It seems that works as some kind of error handling. I guess that we can return the page only when some "debug" parameter is sent; and if no debug paramente is sent but it fails, just return a status code of 500 or 404. What do you think?

The page was there simply as a landing page for GA, such that when you setup the account you can point to it and let GA verify that your tracker is "installed".

I think the new behavior we want is:

  • /UA-xxxxxxx - shows account template page; same behavior as now.
  • /UA-xxxxxxx/page - creates a hit with page path set to "/page"
  • /UA-xxxxxxx/page?useReferrer - set page path to value in Referrer header, and if Referrer is missing then fallback to "/page"

The above is different from what I suggested earlier, but I think it works for your use case. The benefit is that we always have a fallback value, even if Referrer is missing.

Does that make sense?

@Guisardo
Copy link
Contributor Author

Hi @igrigorik , thanks for the response.
Yes, I like that fallback approach.
Can you handle that improve? If not, i'll try to do it on next weekend

@igrigorik
Copy link
Owner

@Guisardo been swamped. If you have bandwidth, please take a run at it.

@Guisardo
Copy link
Contributor Author

Guisardo commented Apr 1, 2016

Hi @igrigorik , can you check this again? feel free to modify any part that you think should need improvements.
Best,

@@ -101,12 +102,23 @@ func handler(w http.ResponseWriter, r *http.Request) {
return
}

// activate referrer path if ?useReferrer is used and if referer exists
if _, ok := query["useReferrer"]; ok {
Copy link
Owner

Choose a reason for hiding this comment

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

I know it's a spelling mistake, but for sake of consistency let's use useReferer (one "r") to align with the actual header?

@igrigorik
Copy link
Owner

@Guisardo thanks for working on this. Overall, I think this is good. A few comments/nits above.

@Guisardo
Copy link
Contributor Author

Guisardo commented Apr 9, 2016

Hi @igrigorik , I just commit the changes you requested.
Best,

@igrigorik igrigorik merged commit dd96cd4 into igrigorik:master Apr 9, 2016
@igrigorik
Copy link
Owner

👍 thanks for working on this!

@Monday542
Copy link

I need help and please someone scam me please

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

Successfully merging this pull request may close these issues.

5 participants