ethercreative / seo

SEO utilities including a unique field type, sitemap & redirect manager
MIT License
276 stars 95 forks source link

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

Closed jacotijssen closed 1 year ago

jacotijssen commented 2 years ago

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

jacotijssen commented 2 years ago

@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 commented 2 years ago

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

SoftboiledStudios commented 2 years ago

+1 to merge PR!!

jannisborgers commented 2 years ago

+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 commented 2 years ago

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

rydncn commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 1 year ago

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.

ItagMichael commented 1 year ago

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 commented 1 year ago

+1

jonathanpardon commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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

remcoov commented 1 year ago

Yep, #447.

brandonkelly commented 1 year ago

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

remcoov commented 1 year ago

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

bjprodneyl commented 1 year ago

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.