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

Set referer for getting referring page title to nothing, then avoid fetching empty referers #92

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

benlk
Copy link
Collaborator

@benlk benlk commented Oct 30, 2020

Changes

This pull request makes the following changes:

  • When looking up the referer for a hit to the tracking pixel, don't include a referer in the lookup
  • If the referer URL is empty, avoid parsing it for a URL host and URL path.

Why

For #80 and #91, though I'm not sure that this does actually address the DOS issue. It will prevent the tracker from firing a request if the referer is not set, and will prevent the tracker from tracking hits without a referer.

Testing/Questions

Features that this PR affects:

  • the tracker pixel's referer lookup

Questions that need to be answered before merging:

  • Is this PR targeting the correct branch in this repository?
  • Does it work?

Steps to test this PR:

  1. Set up a Republication site, including Google Analytics
  2. View your site's access log, or
  3. Copy the republication text, including image, and put that HTML in a file on a server somewhere. It can be a localhost page or a .test development server or your-private-site.example.org or a GitHub page or a live-edited browser. The HTML from the republication text needs to be rendered so that the image is fetched.
  4. View that HTML in a browser.
  5. Open the republished article's edit view, and view the number of hits it received. It should have received one, from the URL that you hosted it. Same with your Google Analytics.
  6. Extract the tracking pixel img's src="" URL, and load that URL directly in your browser.
  7. You should not see any new hits for an entry in the hits table for a row with no source URL. Same with your Google Analytics.

Additional information

Requested by the Center for Public Integrity.

@benlk benlk changed the title 91 tracking pixel dos Set referer for getting referring page title to nothing, then avoid fetching empty referers Oct 30, 2020
@jonathanstegall
Copy link
Contributor

Is it best to wait for this to get merged before using this plugin? I'm finally getting around to hopefully launching it soon.

@benlk
Copy link
Collaborator Author

benlk commented Dec 2, 2020

Probably yes. You could maintain a private fork of the plugin that has this branch merged.

@benlk benlk added category: feature request New/added features community contribution From outside contributors bug and removed category: feature request New/added features labels Feb 9, 2021
@benlk benlk requested a review from a team as a code owner May 24, 2022 10:19
@adekbadek
Copy link
Member

Hi @benlk – could you please resolve the conflicts here?

@benlk
Copy link
Collaborator Author

benlk commented Jun 8, 2022

@adekbadek Resolved, but please re-review this PR as it's been a long while since I worked on this code in earnest.

@benlk
Copy link
Collaborator Author

benlk commented Nov 5, 2024

Note that this has significant merge conflicts now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug community contribution From outside contributors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants