Skip to content

Change X-Robots-Tag header from "none" to "noindex, nofollow" - #36689

Merged
szaimen merged 1 commit into
masterfrom
fix/x-robots-tag-header
Feb 16, 2023
Merged

Change X-Robots-Tag header from "none" to "noindex, nofollow"#36689
szaimen merged 1 commit into
masterfrom
fix/x-robots-tag-header

Conversation

@MichaIng

@MichaIngMichaIng commented Feb 13, 2023

Copy link
Copy Markdown
Member

Summary

While "none" is indeed equivalent to "noindex, nofollow" for Google, but seems to be not supported by Bing and probably other search engines.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/meta/name#other_metadata_nameshttps://developers.google.com/search/docs/crawling-indexing/robots-meta-tag?hl=de#comma-separated-listhttps://www.bing.com/webmasters/help/which-robots-metatags-does-bing-support-5198d240

TODO

  • Check docs for equivalent change

Checklist

@MichaIng

Copy link
Copy Markdown
MemberAuthor

/backport to stable25

@MichaIng

Copy link
Copy Markdown
MemberAuthor

/backport to stable24

Comment threadcore/js/setupchecks.js
@MichaIng
MichaIngforce-pushed the fix/x-robots-tag-header branch from 78d6908 to ad2c43bCompareFebruary 13, 2023 13:43
@MichaIng

Copy link
Copy Markdown
MemberAuthor

This btw means that all admin, who did not have a warning before, do now see it, until their .htaccess or webserver config has been adjusted. Since some are sensitive about "being annoyed" by warnings, we might want to do something about it, though not sure what.

  • There is no way to update the .htaccess on Nextcloud update automatically, is it?
  • We could allow the none value as well, although knowing that it is ignored by Bing, this wouldn't be responsible.
  • Show a different message if the current value is none, to better explain why this warning shows up now and not before? The special treatment could be removed after 1-2 Nextcloud versions, when we assume that every admin has seen it.

@szaimen

Copy link
Copy Markdown
Contributor
  • There is no way to update the .htaccess on Nextcloud update automatically, is it?

Actually the .htaccesss is delivered with the Nextcloud archive so it should usually be overwritten with the new one upon updating.

@MichaIng

Copy link
Copy Markdown
MemberAuthor

The bottom section is however a custom one and preserved on update, so it is not just replaced. I actually never check whether the top section is updated.

@szaimen

Copy link
Copy Markdown
Contributor

The bottom section is however a custom one and preserved on update, so it is not just replaced.

It is not preserved upon update IIRC.

@MichaIng

Copy link
Copy Markdown
MemberAuthor

It is, or otherwise occ maintenance:update:htaccess (respectively the steps behind it) must run afterwards, since the directives for error documents and pretty URL (omitted index.php) with custom rewrite base (sub directory in my case) are always preserved here.

@szaimen

szaimen commented Feb 13, 2023

Copy link
Copy Markdown
Contributor

I see. I thought the file would always get updated since it must have the latest changes...

@MichaIng

Copy link
Copy Markdown
MemberAuthor

Quite possible (and would be reasonable) if the bottom section of the old .htaccess would be appended to the new one, that way updating possibly required changes but preserving setup-depending and custom changes.

MichaIng added a commit to MichaIng/DietPi that referenced this pull request Feb 13, 2023
- DietPi-Software | Change X-Robots-Tag header from "none" to "noindex, nofollow": nextcloud/server#36689
@nickvergessen
nickvergessen removed their request for review February 14, 2023 11:30
@nickvergessen

Copy link
Copy Markdown
Member

Not my area of expertise, so removing my review as I'm overloaded at the moment

@kesselbkesselb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you 👍

Comment threadcore/js/setupchecks.js

@ChristophWurstChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not an expert here either but looks good

Nginx doc should definitely be updated and this needs to be mentioned in the admin doc changes of 27

Comment thread.htaccess
@ChristophWurstChristophWurst added the pending documentation This pull request needs an associated documentation update label Feb 15, 2023
@MichaIng

MichaIng commented Feb 15, 2023

Copy link
Copy Markdown
MemberAuthor

The .htaccess is indeed somehow radically updated. The upper part, as expected, but also custom changes below the #### DO NOT CHANGE ANYTHING ABOVE THIS LINE #### line are lost, at least this happened to some comments I added there for testing. Since the base path depending redirects are preserved, occ maintenance:update:htaccess is called, respectively the internal code behind it.

This means Apache users with AllowOverride All won't see any warning, but others will do. And all Nextcloud appliances of course need to react.

@st3iny

Copy link
Copy Markdown
Member

Integration test failures do not seem to be related:

 �[31mAnd modify LDAP configuration�[39m �[30m# LDAPContext::modifyLDAPConfiguration()�[39m
�[31m | ldapExpertUsernameAttr | uid |�[39m
�[31m Server error: `PUT http://localhost:8080/ocs/v2.php/apps/user_ldap/api/v1/config/s01` resulted in a `500 Internal Server Error` response:
<?xml version="1.0"?>
<ocs>
<meta>
<status>failure</status>
<statuscode>500</statuscode>
<message>Internal Server (truncated...)
(GuzzleHttp\Exception\ServerException)�[39m

@ChristophWurstChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish bug and removed 3. to review Waiting for reviews labels Feb 16, 2023
backportbot-nextcloudBot pushed a commit to nextcloud/documentation that referenced this pull request Feb 16, 2023
For reference: nextcloud/server#36689
Signed-off-by: MichaIng <micha@dietpi.com>
backportbot-nextcloudBot pushed a commit to nextcloud/documentation that referenced this pull request Feb 16, 2023
For reference: nextcloud/server#36689
Signed-off-by: MichaIng <micha@dietpi.com>
@szaimen

Copy link
Copy Markdown
Contributor

CI failure unrelated

@DaphneMuller

Copy link
Copy Markdown
Contributor

hello @MichaIng
Thank you so much for your work on this pull request! This ticket seems to have the tag 'missing documentation'.
Is there any chance you could clarify what documentation is missing? Is this for admins or for app developers?

@MichaIng

Copy link
Copy Markdown
MemberAuthor

Hi Daphne, the documentation change has been merged already, so not pending anymore: nextcloud/documentation#9635

@MichaIngMichaIng removed the pending documentation This pull request needs an associated documentation update label Feb 21, 2023
@DaphneMuller

Copy link
Copy Markdown
Contributor

thank you for your lightning fast reply and for removing the pending documentation label @MichaIng !❤️

dotlambda added a commit to dotlambda/nixpkgs that referenced this pull request Mar 26, 2023
Upstream did so in nextcloud/server#36689 and
Nextcloud now complains that
The "X-Robots-Tag" HTTP header is not set to "noindex, nofollow".
This is a potential security or privacy risk, as it is recommended
to adjust this setting accordingly.
github-actionsBot pushed a commit to NixOS/nixpkgs that referenced this pull request Mar 26, 2023
Upstream did so in nextcloud/server#36689 and
Nextcloud now complains that
The "X-Robots-Tag" HTTP header is not set to "noindex, nofollow".
This is a potential security or privacy risk, as it is recommended
to adjust this setting accordingly.
(cherry picked from commit 15b859c)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to releaseReady to be released and/or waiting for tests to finishbugenhancementprivacy

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for multiple X-Robots-Tags

7 participants

@MichaIng@szaimen@nickvergessen@st3iny@DaphneMuller@ChristophWurst@kesselb