Re: [Savannah-hackers-public] vcs0 cgi errors ("502 bad gateway")

[Bob Proulx]

Hi Assaf,

Assaf Gordon wrote:
>    2017/02/10 10:58:20 [error] 3647#0: *370418 connect() to 
> unix:/var/run/fcgiwrap.socket failed (11: Resource temporarily unavailable) 
> while connecting to upstream, client: X.X.X.X,  server: git.savannah.gnu.org, 
> request: "GET /r/freetype/freetype2.git/info/refs?service=git-upload-pack 
> HTTP/1.1", upstream: "fastcgi://unix:/var/run/fcgiwrap.socket:", host: 
> "git.savannah.nongnu.org"
> I've restarted fcgiwrap and reloaded nginx, and it seems to work now.

Hmm...  I don't know why that would happen or why restarting it would
be needed.  I will put setting up a 'monit' check for it on my list.

> Also, the user reported that this URL gives "404 not found":
>    http://git.savannah.gnu.org/gitweb
> 
> that's because our nginx confs expects a trailing slash.
> I've added a redirection in /etc/nginx/sites-available/git .

I am the cause for the problem at a few different levels.  Originally
when I set up the location entries for Let's Encrypt I used the bad
syntax of a location without a trailing slash.

  location /.well-known { root /home/certbot/www; }  # <- Bad, do not use

And then after that all of us copied that syntax everywhere.  And so
was the cause for it at the later levels too.  I shouldn't say "bad"
maybe because it worked for the purpose.  But it had weird
interactions with other entries in other contexts.  A common substring
match when combined with matching URLs.

When moving git over (again) this week I created the regression with
the smart http transport.  In that it was smart on the old and dumb on
the new.  I had not realized that the smart transport used a cgi-bin
program git-http-backend and that is how the smart function worked.
Figured that out and installed it.

Then I went through all of the combinations trying to make sure
everything was working and updating the savannah-test project entries
try to cover all of the possibilities.  Doing that exposed problems
with the robots.txt file.  (Thank goodness for tests!  I wouldn't have
found that so quickly if it weren't for tests!)

So here is the interaction.  Using "location /r { ...stuff... }"
breaks the robots.txt file.  It turns out that Nginx matches /r to
/robots.txt and therefore sent all robots.txt requests off to the
newly installed git-http-backend which doesn't handle general web
requests for things like a robots.txt file.

Researching that led me to a learning about Nginx.  All location
blocks should always end with a trailing slash.

  location /.well-known/ { root /home/certbot/www; }  # a good entry

We never tried anything that used "/.well-knownfoo" or we would have
found that the common substring match hit there too and did the wrong
thing.  It was a bug that didn't hit until "/r" hit "/robots.txt" and
did the wrong thing.

Therefore I updated all of the entries to have a trailing slash
globally so as to be correct and to avoid future problems with common
substring matches.

Then while working through all of the tests for the sites I ran into
detail that hitting a URL without a trailing slash does not match
anything and returns a 404 not found.  Yet it worked when we were
using a proxy configuration.  Why did one work and another not work?
Here is why.  Docs for reference:

  http://nginx.org/en/docs/http/ngx_http_core_module.html#location

  If a location is defined by a prefix string that ends with the slash
  character, and requests are processed by one of proxy_pass,
  fastcgi_pass, uwsgi_pass, scgi_pass, or memcached_pass, then the
  special processing is performed.  In response to a request with URI
  equal to this string, but without the trailing slash, a permanent
  redirect with the code 301 will be returned to the requested URI with
  the slash appended.

So proxies are special.  They always work by the internal rule above.
But non-proxy blocks are not and therefore do not.  Docs above go on
to say this following:

  If this is not desired, an exact match of the URI and location could
  be defined like this:

  location /user/ {
      proxy_pass http://user.example.com;
  }

  location = /user {
      proxy_pass http://login.example.com;
  }

Note the use of "=" matching for an exact match.  That is the specific
thing to note there.  That example shows working with proxies.  But it
applies just as well to the non-proxy blocks too.  Therefore I added
the same type of configuration.  This worked well:

        location = /r { return 302 $request_uri/; }
        location /r/ {
               autoindex on;
                alias /srv/git/;
                location ~ /r(/.*/(info/refs|git-upload-pack)$) {
                        gzip off;
                        include fastcgi_params;
                        fastcgi_pass unix:/var/run/fcgiwrap.socket;
                        fastcgi_param SCRIPT_FILENAME
                        /usr/local/sbin/git-http-backend;
                        fastcgi_param PATH_INFO $1;
                        fastcgi_param GIT_HTTP_EXPORT_ALL true;
                        fastcgi_param GIT_PROJECT_ROOT /srv/git;
                        client_max_body_size 0;
                }
        }

With that working I worked through all of the test failures and set up
all of the other redirects as seemed appropriate.

My test cases for were like this:

  http://git.savannah.gnu.org/gitweb/

Not like this without the trailing slash:

  http://git.savannah.gnu.org/gitweb

And at the time I was wondering the other cases.  I mean pedantically
the location really, really is http://git.savannah.gnu.org/gitweb/
with a trialing slash and not without.  At one level I think without
it really should be a 404 since that does exist.  However I am all for
helping people do the right thing with some typing aides and this is
just another one of those times where this is a typing aide.

In this case we don't want to add the case without the = match.

  location /gitweb

That would mean that someone doing /gitwebnew or some such would hit
and get the redirect.  That isn't desired.  I modified your fix today
to be = matching with:

  location = /gitweb

And I am updating my tests to include *yet another test case* for this
both with and without trailing slashes for every one of the test case
entries.  That pretty much doubles the runtime of the test.  Oh well.
What good are the tests unless we try each different combination.  Sigh.

I added the explicit redirects even for internal paths that are not
typed by users.  And for proxied blocks where that is the implicit
behavior.  This way anyone doing copy and pasting will get the right
answer regardless.

Bob