ssg icon indicating copy to clipboard operation
ssg copied to clipboard

TrustedProxies causes issue generating pages with custom routes

Open tao opened this issue 4 years ago • 13 comments

I encounter an error with SSG when generating custom pages defined in the routes files, the issue is caused by TrustedProxies middleware in Laravel.

On my site I have a custom view generated for an index for each year.

Route::statamic('articles/{year}', 'articles.years.show', [
    'layout' => 'default',
])->where('year', '[0-9]+');

In my SSG config, I loop through the article years from the start date to the current year and manually include these pages to be generated:

<?php

for ($year = 1972; $year <= date("Y"); $year ++) {
    $yearIndexes[] = "/articles/{$year}";
}

return [

    ...

    /*
    |--------------------------------------------------------------------------
    | Additional URLs
    |--------------------------------------------------------------------------
    |
    | Here you may define a list of additional URLs to be generated,
    | such as manually created routes.
    |
    */

    'urls' => array_merge(
        [
            // other urls to include
        ],
        $yearIndexes,
    ),

When I generate the static website, I encounter this error:

[✔] /articles
Generating /articles/1972...

   TypeError

  Argument 2 passed to Symfony\Component\HttpFoundation\IpUtils::checkIp4() must be of the type string, null g
iven, called in /Users/tao/Code/www.statamic/vendor/symfony/http-foundation/IpUtils.php on line 46


  at vendor/symfony/http-foundation/IpUtils.php:62
     58▕      * @param string $ip IPv4 address or subnet in CIDR notation
     59▕      *
     60▕      * @return bool Whether the request IP matches the IP, or whether the request IP is within the CI
DR subnet
     61▕      */
  ➜  62▕     public static function checkIp4(?string $requestIp, string $ip)
     63▕     {
     64▕         $cacheKey = $requestIp.'-'.$ip;
     65▕         if (isset(self::$checkedIps[$cacheKey])) {
     66▕             return self::$checkedIps[$cacheKey];

      +51 vendor frames
  52  please:37
      Illuminate\Foundation\Console\Kernel::handle(Object(Symfony\Component\Console\Input\ArgvInput), Object(S
ymfony\Component\Console\Output\ConsoleOutput))

I can overcome this error by commenting out the Laravel middleware... but this isn't ideal and wondering if there's a more elegant way to solve this in the SSG code.

For reference this is my TrustedProxies.php file:

class TrustProxies extends Middleware
{
    /**
     * The trusted proxies for this application.
     *
     * @var array
     */
    protected $proxies = '*';

    /**
     * The headers that should be used to detect proxies.
     *
     * @var string
     */
    protected $headers = Request::HEADER_X_FORWARDED_ALL;
}

tao avatar Dec 01 '20 11:12 tao

I've created an example repo with this issue.

protected $proxies;

It works fine until you change this line to this:

protected $proxies = '*';

tao avatar Dec 30 '20 21:12 tao

Not saying this can't be fixed, but I'm curious what situation you're in that you need to use a static site and trusted proxies.

jasonvarga avatar Apr 22 '21 21:04 jasonvarga

It's been a couple months so I'm having trouble remembering exactly but it might have come up when I moved my SSG action to a github pipeline because it was slow on my personal computer? And then every night I generate the new pages that have changed in the last 24 hours and re-deploy it to S3.

There is a small chance it did occur on localhost but I see there's been a few merges to SSG this week so I'll review the updates this weekend and see if it's still causing an issue.

tao avatar Apr 22 '21 23:04 tao

I would have thought you'd need to tinker with trusted proxies if you're hosting on somewhere like AWS. If your site exists just to generate a static version, I wouldn't think it was necessary.

It definitely still breaks if I change to protected $proxies = '*'; but I'm wondering if I need to spend effort fixing it if you don't actually need to play with your trusted proxies.

jasonvarga avatar Apr 23 '21 14:04 jasonvarga

I took another look at this... I don't really understand what's causing this issue, especially since (from the example repo I created before) it seems to happen on localhost too. However, this trusted proxies issue is something that happens quite often on Laravel deployments on hosting like Heroku, so it's something I'm sure most developers are familiar with. When I tested it again it generates the / home page but fails on the second /404 page 😕

Maybe it's related somehow to the way SSG does a request for the pages that creates the issue?

I think for now we can probably close this issue and open it up again in the future if it causes an issue for anyone else.

tao avatar Apr 27 '21 21:04 tao

I guess what I'm saying is that if you're hosting your non-static site on Heroku and need to use trusted proxies, sure, that's fine. But then are you also generating the static site on Heroku?

Wouldn't the static version of the site be production?

Is the non-static site you have on Heroku your staging site?

Maybe it's related somehow to the way SSG does a request

It's definitely this. We fake certain parts of the request as if you were requesting the page being generated. There's probably a couple more parts of the request we need to fudge in order for the trusted proxies stuff to work.

jasonvarga avatar Apr 28 '21 03:04 jasonvarga

The static site is our production site and it's hosted as files on CloudFront. We generating it with SSG each night on a GitHub pipeline. However, the bug occurs (with this example repo) on my local machine and I'm just using MacOS & Valet.

I'll be playing with SSG a lot more soon as we're close to launching the site so I'll continue testing (without changing my trusted proxies) and see if I can narrow down the cause of the issue.

tao avatar Apr 28 '21 09:04 tao

Hi @tao , are you able to find a workaround for this?

abuhurairah-aar avatar Oct 05 '21 15:10 abuhurairah-aar

@abuhurairah-aar I haven't had any recent issues with this but I'm not using Heroku anymore either.

I am using $proxies; instead of $proxies = '*'; in my code.

tao avatar Oct 05 '21 16:10 tao

@tao Thanks for the feedback. I've solved the issue by reverted back my $proxies = '**'; to $proxies; like you mentioned, but since my application behind a load balancer, I still need to use $proxies='**' so that I able to configure https properly. Then, whenever I want to run php please ssg:generate i will revert to $proxies; manually. It is cumbersome, but I guess this is the solution for now. Hopefully there's a way to dynamically setup the $proxies through .env file, but I got no luck trying that out yet.

abuhurairah-aar avatar Oct 13 '21 07:10 abuhurairah-aar

I can confirm this behaviour.

jonassiewertsen avatar Sep 09 '22 12:09 jonassiewertsen

The request passed in here does have a null value, which does resolve in the shown error.

The code below does show a workaound.

https://github.com/statamic/ssg/compare/master...spiegeltechlab:ssg:trusted-proxy

@jasonvarga

  1. Do you see any security concerns?
  2. I think it should be configurable (ip and if activated at all)
  3. There might be a way which is more clever, but this is the fasted solution I could find.

@tao and @abuhurairah-aar does this workaround solve your issue as well?

After some feedback, I can create a PR of course.

jonassiewertsen avatar Sep 09 '22 15:09 jonassiewertsen

I also faced the same issue yesterday and I can confirm @jonassiewertsen 's workaround solves the issue

phyroslam avatar Sep 09 '22 16:09 phyroslam

Thanks @jonassiewertsen, your patch fixes the issue. Would be super happy about a PR which Jason can review then. :)

o1y avatar Feb 20 '23 11:02 o1y

After getting feedback to my questions (see above), I am happy to create a PR.

jonassiewertsen avatar Feb 20 '23 12:02 jonassiewertsen

@jonassiewertsen Did this PR ever get created / merged? I can't see it anywhere and I'm still experiencing this problem.

I'm patching manually for now but have to reapply every time I upgrade.

jegardiner avatar Aug 22 '23 08:08 jegardiner

I'm patching manually for now but have to reapply every time I upgrade.

This PR has not been merged yet. You could use composer-patches. This is what I'm doing to have this patch applied everytime I run composer updates.

o1y avatar Aug 22 '23 08:08 o1y

I'm patching manually for now but have to reapply every time I upgrade.

This PR has not been merged yet. You could use composer-patches. This is what I'm doing to have this patch applied everytime I run composer updates.

Thanks. I will give that a try.

Hopefully the PR will be merged soon.

jegardiner avatar Aug 22 '23 08:08 jegardiner

No feedback from the core team yet, but I created that PR anyways. I hope it does help.

https://github.com/statamic/ssg/pull/145

jonassiewertsen avatar Aug 22 '23 12:08 jonassiewertsen