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

Environment check for production always returns false if ENVIRONMENT is not set #432

Closed
jacotijssen opened this issue Sep 15, 2022 · 21 comments · Fixed by #434
Closed

Environment check for production always returns false if ENVIRONMENT is not set #432

jacotijssen opened this issue Sep 15, 2022 · 21 comments · Fixed by #434

Comments

@jacotijssen
Copy link

jacotijssen commented Sep 15, 2022

Description

Since Craft 4, all environment variables have been prefixed with CRAFT_. So ENVIRONMENT became CRAFT_ENVIRONMENT.
In the SeoService.php there is a check for both variables:

$env = getenv('ENVIRONMENT') ?? getenv('CRAFT_ENVIRONMENT');

// Always noindex except on production environment
if ($env !== 'production')
{
	$headers->set('x-robots-tag', 'none, noimageindex');
	return;
}

However, if an environment variable isn't set, the getenv() method will return false.
The null coalescing operator will only work on NULL, so if ENVIRONMENT isn't set, the $env will always be false.
So basically, every Craft 4 website using default configuration will always have the x-robots-tag header set to 'none, noimageindex',

I will make a pull request to fix this.

Steps to reproduce

  1. Use Craft 4 using the new environment variables, prefixed with CRAFT_
  2. The X-Robots-Tag header will always be "'none, noimageindex'"

Additional info

  • Craft version: 4.*
  • SEO version: *
  • PHP version: *
@jacotijssen
Copy link
Author

@Tam, @ethercreative I know you guys are busy, but this seems to cause a lot of issues for several people. I think some people might not even know their site is not indexable anymore, which can cause a lot of damage. Is it possible to take a look at this?

@ewanduthie
Copy link

@ethercreative +1 to merge this PR please! We have had two sites affected by the same.

@SoftboiledStudios
Copy link

+1 to merge PR!!

@jannisborgers
Copy link

+1, please merge this PR 👍

Today I noticed this issue several weeks after a new site launched and countless times of re-validating in Search Console (which unfortunately doesn't give a reason for not indexing the pages). The only option right now is disabling the plugin alltogether, as the issue is buried in the plugin code. This will also remove the sitemap.xml, which is the reason I use this plugin on this project.

@cstudios-slovakia
Copy link

Is there any hotfix we can apply? One of our clients just lost all of its sites from Google.

@rydncn
Copy link

rydncn commented Nov 24, 2022

@cstudios-slovakia we have a site currently on Craft v4.3.3 and the plugin is working by adding ENVIRONMENT alongside CRAFT_ENVIRONMENT in the .env file. Hope this helps as a hotfix for everyone until this is sorted.

CRAFT_ENVIRONMENT=production
ENVIRONMENT=production

@cstudios-slovakia
Copy link

Here is hotfix for the template, but it does not solve the bug, just suppresses the symptoms:

{% if craft.app.config.env == 'production' %}
   {% header "X-Robots-Tag: all" %}
{% else %}
   {% header "X-Robots-Tag: noindex, nofollow, none" %}
{% endif %}

@SoftboiledStudios
Copy link

Here is hotfix for the template, but it does not solve the bug, just suppresses the symptom

Here is hotfix for the template, but it does not solve the bug, just suppresses the symptoms:

{% if craft.app.config.env == 'production' %}
   {% header "X-Robots-Tag: all" %}
{% else %}
   {% header "X-Robots-Tag: noindex, nofollow, none" %}
{% endif %}

Were exactly do you put this then?

@jannisborgers
Copy link

@cstudios-slovakia we have a site currently on Craft v4.3.3 and the plugin is working by adding ENVIRONMENT alongside CRAFT_ENVIRONMENT in the .env file. Hope this helps as a hotfix for everyone until this is sorted.

CRAFT_ENVIRONMENT=production
ENVIRONMENT=production

This works in Craft 4.3.1 with SEO plugin 4.0.3.

@ryanmasuga
Copy link

We just discovered this today - wondering why all of one client's sites simply disappeared from Google's index.

Please merge one of the pull requests.

darrylhein added a commit to xmmedia/starter_craft that referenced this issue Dec 7, 2022
@ItagMichael
Copy link

I understand this is a free plugin and I hate to say it, but is there a reason why this hasn't been fixed? The issue seems quite serious and easy to miss until it is too late for those unaware. Easy fixes have been proposed, is there a reason why one cannot be merged?

@P4KM4N
Copy link

P4KM4N commented Jan 27, 2023

+1

@jonathanpardon
Copy link

I confirm, the check for the environment is not right in the SEO source - SeoService.

$env = getenv('ENVIRONMENT') ?? getenv('CRAFT_ENVIRONMENT');

==>> getenv('ENVIRONMENT') with a default craft cms .env config (with no ENVIRONMENT config), will always return false (not null), so in the lines bellow will always set the x-robots-tag to 'none,noimageindex'.

// Always noindex except on production environment
if ($env !== 'production')
{
	$headers->set('x-robots-tag', 'none, noimageindex');
	return;
}

===>> the fix would be reworking the ?? to a check on a false value

So indeed a quick fix is to add ENVIRONMENT=production to your .env, but hope this will be fixed in the future.
ENVIRONMENT=production
CRAFT_ENVIRONMENT=production

@ryanmasuga
Copy link

ryanmasuga commented Mar 8, 2023

Does anyone even maintain this anymore? This is a big enough issue (that is easy enough to fix) and is not being addressed that I clicked "Report plugin" in the Craft Plugin store (right side: https://plugins.craftcms.com/seo) to see if the Craft devs can figure out if this developer is still around - or warn users it may not be actively maintained.

@jacotijssen
Copy link
Author

jacotijssen commented Mar 8, 2023

Does anyone even maintain this anymore?

@ryanmasuga well, with 118 open issues and no updates in the last 9 months, I think we all know the answer to that question 😥

Since this is a open source plugin, I find it very difficult to "report" them, especially because (apart from this issue) I'm very grateful for this plugin. But I also think this might be the only thing left to do.

So I joined you in reporting the plugin and hopefully everyone else here will do the same.

@ryanmasuga
Copy link

ryanmasuga commented Mar 8, 2023

@jacotijssen It would be nice thing to warn potential users on the Craft Plugin store page that this may be abandoned (I think they do that?). It's got over 16,000 active installs - and really shouldn't be adding to that number with issues like this one that may catch people unaware.

(I've been notified that Craft is reaching out to the developer, and if they don't hear back in a specified time, they mark it as abandoned.)

@pascalminator
Copy link

So... the plugin is now officially abandoned. Shame because it was the only free SEO plugin for Craft.

@remcoov
Copy link

remcoov commented Apr 6, 2023

Yep, #447.

@ryanmasuga ryanmasuga mentioned this issue Apr 6, 2023
@brandonkelly
Copy link
Contributor

The plugin should be using Craft::$app->env, rather than worrying about how it was set exactly.

@jacotijssen jacotijssen changed the title Enviroment check for production always returns false if ENVIRONMENT is not set Environment check for production always returns false if ENVIRONMENT is not set Apr 7, 2023
@remcoov
Copy link

remcoov commented May 11, 2023

This was supposed to be fixed in 4.1.0, but we're still getting x-robots-tag: none on production?

@bjprodneyl
Copy link

This was supposed to be fixed in 4.1.0, but we're still getting x-robots-tag: none on production?

Same here. Seems like I had it working by adding a robots.txt file to my server, but after the update it is showing noindex tag in pages again.

@Tam Tam closed this as completed in a9a6c9c May 15, 2023
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 a pull request may close this issue.