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.

Sign up for GitHub

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

Jump to bottom

Set response code for ads.txt requests #14174

Merged
merged 1 commit into from
Dec 5, 2019
Merged

Set response code for ads.txt requests #14174

merged 1 commit into from
Dec 5, 2019

Conversation

hastinbe
Copy link

@hastinbe hastinbe commented Dec 4, 2019

Fixes #14169

Changes proposed in this Pull Request:

Sends an HTTP 200 OK response code for /ads.txt requests. Previously no response code was set.

Testing instructions:

  • Make a request for domain.com/ads.txt
  • When Ads are enabled, the generated ads.txt should be returned with a 200 HTTP response code.

Proposed changelog entry for your changes:

  • Set response code of 200 OK for ads.txt requests

Set response code for ads.txt requests
1867240 
Under some configurations, the web server will set a status code of 404 for requests to domain.com/ads.txt since the physical file cannot be present for WordAds to return an automatically generated ads.txt. When the generated response is sent, no response code was set so the previous 404 is used. This will cause crawlers such as Google's to ignore the response and the file will be considered non-existent.

Fixes #14169
@hastinbe hastinbe requested a review from a team December 4, 2019 22:11
@jetpackbot
Copy link

Warnings
⚠️

The PR is missing at least one [Status] label. Suggestions: [Status] In Progress, [Status] Needs Review

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 1867240

@kraftbj kraftbj added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Type] Bug When a feature is broken and / or not performing as intended [Feature] WordAds labels Dec 4, 2019
@kraftbj
Copy link
Contributor

kraftbj commented Dec 4, 2019

Noting that /ads.txt is reported as a 404 for my personal site, which this would correct to 200.

@kraftbj kraftbj added this to the 8.1 milestone Dec 4, 2019
kraftbj
kraftbj approved these changes Dec 4, 2019
View reviewed changes
Copy link
Contributor

@kraftbj kraftbj left a comment

Choose a reason for hiding this comment

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

Works as expected on my site. All versions of PHP should have http_response_code. Approving.

Thanks @hastinbe for the report and the fix!

@kraftbj kraftbj added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Dec 4, 2019
@jeherve jeherve merged commit 8dbad54 into Automattic:master Dec 5, 2019
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Dec 5, 2019
jeherve added a commit that referenced this pull request Dec 13, 2019
@jeherve
Changelog: add #14174
9587e45
zinigor added a commit that referenced this pull request Dec 30, 2019
@jeherve @zinigor
Changelog: 8.1 additions (#14166)
fff089c 
* Changelog: 8.1 additions

* Changelog: add #13858

* Changelog: add #13963

* Changelog: add #14174

* Changelog: add #14178

* Changelog: add #14175

* Changelog: add #14192

* Changelog: add #14196

* Changelog: add #14182

* Changelog: add #14218

* Changelog: add #14214

* Changelog: add #13757

* Changelog: add #14190

* Changelog: add #14131

* Changelog: add #14101

* Changelog: add #14203

* Changelog: add #14211

* Changelog: add #14224

* Changelog: add #14230

* Changelog: add #14241

* Changelog: add #14249

* Changelog: add #14264

* Changelog: add #14263

* Changelog: add #14256

* Changelog: add #10189

* Changelog: add #14240

* Changelog: add #14239

Also added some new entries to the testing file.

Co-authored-by: Igor Zinovyev <zinigor@gmail.com>
zinigor added a commit that referenced this pull request Dec 30, 2019
@jeherve @zinigor
Changelog: 8.1 additions (#14166)
d4a7565 
* Changelog: 8.1 additions

* Changelog: add #13858

* Changelog: add #13963

* Changelog: add #14174

* Changelog: add #14178

* Changelog: add #14175

* Changelog: add #14192

* Changelog: add #14196

* Changelog: add #14182

* Changelog: add #14218

* Changelog: add #14214

* Changelog: add #13757

* Changelog: add #14190

* Changelog: add #14131

* Changelog: add #14101

* Changelog: add #14203

* Changelog: add #14211

* Changelog: add #14224

* Changelog: add #14230

* Changelog: add #14241

* Changelog: add #14249

* Changelog: add #14264

* Changelog: add #14263

* Changelog: add #14256

* Changelog: add #10189

* Changelog: add #14240

* Changelog: add #14239

Also added some new entries to the testing file.

Co-authored-by: Igor Zinovyev <zinigor@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kraftbj kraftbj kraftbj approved these changes

Assignees
No one assigned
Labels
[Feature] WordAds [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
8.1
Development

Successfully merging this pull request may close these issues.

Ads doesn't set status code for ads.txt
5 participants
@hastinbe @jetpackbot @kraftbj @jeherve @matticbot