Allow 'Z' in sign URLs. (#317)

Fixes breakage from #313 (9c34d5e).

Also, when "fetch/sign URLs do not match config", log the underlying
reason(s).

Finally, improve the logging in gateway_server slightly + `go fmt`.

Author: twifkak

cmd/gateway_server/server.go

@@ -34,7 +34,7 @@ var (
 		"Publisher server port.")
 )
 
-type gatewayServer struct{
+type gatewayServer struct {
 	rtvCache *rtv.RTVCache
 }
 
@@ -56,15 +56,17 @@ func createOCSPResponse(cert *x509.Certificate, key crypto.Signer) ([]byte, erro
 	// Construct args to ocsp.CreateResponse.
 	template := ocsp.Response{
 		SerialNumber: cert.SerialNumber,
-		Status: ocsp.Good,
-		ThisUpdate: thisUpdate,
-		NextUpdate: thisUpdate.Add(time.Hour*24*7),
-		IssuerHash: crypto.SHA256,
+		Status:       ocsp.Good,
+		ThisUpdate:   thisUpdate,
+		NextUpdate:   thisUpdate.Add(time.Hour * 24 * 7),
+		IssuerHash:   crypto.SHA256,
 	}
 	return ocsp.CreateResponse(cert /*issuer*/, cert /*responderCert*/, template, key)
 }
 
 func (s *gatewayServer) GenerateSXG(ctx context.Context, request *pb.SXGRequest) (*pb.SXGResponse, error) {
+	log.Println("Handling request with fetchUrl =", request.FetchUrl, "; signUrl =", request.SignUrl)
+
 	certs, err := signedexchange.ParseCertificates(request.PublicCert)
 	if err != nil {
 		return errorToSXGResponse(err), nil
@@ -206,6 +208,6 @@ func main() {
 	var opts []grpc.ServerOption
 	grpcServer := grpc.NewServer(opts...)
 	pb.RegisterGatewayServiceServer(grpcServer, &gatewayServer{rtvCache: rtvCache})
-	fmt.Println("Starting server on port: ", *port)
+	log.Println("Starting server on port: ", *port)
 	grpcServer.Serve(listener)
 }

packager/signer/validation.go

@@ -138,7 +138,7 @@ func isFallbackURLCodePoint(b byte) bool {
 
 	// Vaguely ordered most to least common, to aid short-circuiting:
 	return (b >= 'a' && b <= 'z') || b == '_' || b == '~' ||
-		(b >= '!' && b < 'Z' && b != '"' /*x22*/ && b != '#' /*x23*/ && b != '<' /*x3C*/ && b != '>' /*x3E*/)
+		(b >= '!' && b <= 'Z' && b != '"' /*x22*/ && b != '#' /*x23*/ && b != '<' /*x3C*/ && b != '>' /*x3E*/)
 }
 
 // True iff url matches pattern, as defined by an [URLSet.Sign] block in the
@@ -205,6 +205,8 @@ func parseURLs(fetch string, sign string, urlSets []util.URLSet) (*url.URL, *url
 		// TODO(twifkak): Use errors.Wrap() after changing return types to error.
 		return nil, nil, false, err
 	}
+
+	errs := []string{}
 	for _, set := range urlSets {
 		err := urlsMatch(fetchURL, signURL, set)
 		if err == nil {
@@ -213,8 +215,9 @@ func parseURLs(fetch string, sign string, urlSets []util.URLSet) (*url.URL, *url
 			}
 			return fetchURL, signURL, set.Sign.ErrorOnStatefulHeaders, nil
 		}
+		errs = append(errs, err.Error())
 	}
-	return nil, nil, false, util.NewHTTPError(http.StatusBadRequest, "fetch/sign URLs do not match config")
+	return nil, nil, false, util.NewHTTPError(http.StatusBadRequest, "fetch/sign URLs do not match config; caused by: ", strings.Join(errs, ", "))
 }
 
 // Given a request/response pair for the fetch from the packager to the backend

packager/signer/validation_test.go

@@ -4,6 +4,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"strings"
 	"testing"
 
 	"github.com/ampproject/amppackager/packager/util"
@@ -76,6 +77,15 @@ func TestFetchURLMatches(t *testing.T) {
 		"URL too long")
 }
 
+func TestIsFallbackURLCodePoint(t *testing.T) {
+	// https://url.spec.whatwg.org/#url-code-points + "%", in codepoint order:
+	validURLCodepoints := `!$%&'()*+,-./0123456789:;=?@ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz~`
+	for b := 0; b < 0x100; b++ {
+		expected := strings.ContainsRune(validURLCodepoints, rune(b))
+		assert.Equal(t, expected, isFallbackURLCodePoint(byte(b)), "char: %#v", string(rune(b)))
+	}
+}
+
 func TestSignURLMatches(t *testing.T) {
 	assert.NoError(t, signURLMatches(urlOrDie("https://example.com/"),
 		&util.URLPattern{Domain: "example.com", PathRE: stringPtr(".*"), QueryRE: stringPtr(".*"), MaxLength: 2000}))
@@ -138,7 +148,9 @@ func TestParseURLs(t *testing.T) {
 		{Sign: &util.URLPattern{Domain: "badexample.com", PathRE: stringPtr(".*"), QueryRE: stringPtr(".*"), MaxLength: 2000}},
 	})
 	if assert.NotNil(t, err) {
-		assert.EqualError(t, err, "fetch/sign URLs do not match config")
+		if assert.Error(t, err) {
+			assert.Contains(t, err.Error(), "fetch/sign URLs do not match config")
+		}
 	}
 }