Caddy: httpserver: Trimmed URI doesn't reconstruct query string

[sirkon]

1. What version of Caddy are you using (caddy -version)?

0.10.11

2. What are you trying to do?

Custom simplistic plugin, just to show a error

package main

import (
    "io/ioutil"
    "log"
    "os"

    "net/http"

    "io"

    _ "github.com/mholt/caddy/caddyhttp"

    "github.com/mholt/caddy"
    "github.com/mholt/caddy/caddyhttp/httpserver"
    "github.com/sanity-io/litter"
)

func setup(c *caddy.Controller) error {
    for c.Next() {
        // get configuration
    }
    // now what?
    cfg := httpserver.GetConfig(c)
    cfg.AddMiddleware(func(next httpserver.Handler) httpserver.Handler {
        return handler{}
    })
    return nil
}

// provide loader function
func defaultLoader(serverType string) (caddy.Input, error) {
    contents, err := ioutil.ReadFile("Caddyfile")
    if err != nil {
        if os.IsNotExist(err) {
            return nil, nil
        }
        return nil, err
    }
    return caddy.CaddyfileInput{
        Contents:       contents,
        Filepath:       "Caddyfile",
        ServerTypeName: serverType,
    }, nil
}

func init() {
    caddy.RegisterPlugin(
        "custom",
        caddy.Plugin{
            ServerType: "http",
            Action:     setup,
        },
    )
    httpserver.RegisterDevDirective("custom", "")
    caddy.SetDefaultCaddyfileLoader("default", caddy.LoaderFunc(defaultLoader))
}

type handler struct{}

func (handler) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
    _, err := io.WriteString(w, litter.Sdump(r.URL))
    return http.StatusOK, err
}

func main() {
    caddyfile, err := caddy.LoadCaddyfile("http")
    if err != nil {
        log.Fatal(err)
    }

    instance, err := caddy.Start(caddyfile)
    if err != nil {
        panic(err)
    }

    instance.Wait()
}

3. What is your entire Caddyfile?

0.0.0.0:2015/s {
    custom
}

4. How did you run Caddy (give the full command and describe the execution environment)?

5. Please paste any relevant HTTP request(s) here.

curl 'http://0.0.0.0:2015/s/123213?r87ewfhdghfdfkjghdfkghdfklvckjhvdfs'

6. What did you expect to see?

&url.URL{
  Scheme: "http",
  Opaque: "",
  User: nil,
  Host: "0.0.0.0:2015",
  Path: "/123213",
  RawPath: "",
  ForceQuery: false,
  RawQuery: "r87ewfhdghfdfkjghdfkghdfklvckjhvdfs",
  Fragment: "",
}

7. What did you see instead (give full error messages and/or log)?

&url.URL{
  Scheme: "",
  Opaque: "",
  User: nil,
  Host: "",
  Path: "/123213",
  RawPath: "",
  ForceQuery: false,
  RawQuery: "",
  Fragment: "",
}

8. How can someone who is starting from scratch reproduce the bug as minimally as possible?

Just put the Go code I have pasted into some main.go, then run it with go run main.go

[mholt]

If you check out an earlier version (0.10.10 for example) is the behavior the same?

/cc @tobya

[sirkon]

@mholt it is better and would solve an original problem I started this issue from:

&url.URL{
  Scheme: "",
  Opaque: "",
  User: nil,
  Host: "",
  Path: "/",
  RawPath: "",
  ForceQuery: false,
  RawQuery: "r87ewfhdghfdfkjghdfkghdfklvckjhvdfs",
  Fragment: "",
}

I needed RawQuery and it is lacked in v0.10.11.

Anyway, the parameters of original request.URL should not be lost and it is definitely a bug in the general sense I have posted.

[tobya]

Yes, this is most likely a bug in the code I introduced for #2014 .

I wonder does it affect existing plugins?

[cb22]

@tobya seems like it. I'm using the proxy plugin, and my backend is no longer receiving the query string on 0.10.11. Downgrading to 0.10.10 fixes the problem.

[tobya]

I have submitted a fix.

If you have facility to build and check these issues against code in #2039 that would be great. @cb22 @sirkon

[tobya]

@sirkon Would you be able to confirm if you are getting different values for Host and Scheme between 0.10.10 and this PR?

[sirkon]

Would you be able to confirm if you are getting different values for Host and Scheme between 0.10.10 and this PR?

Well, they are not different. They are the same: both are empty, which is a bug. They should not be empty.

[mholt]

I'm pretty sure the server-side Request struct only gets filled in what is provided in the beginning of the actual HTTP request, which is usually just path: scheme has already been used when it was established with the connection, the Host is in the header, the port is too (and/or is known by the server already), etc.

So missing the query string might have been an issue but I don't think the rest of the fields being empty is a bug.

[sirkon]

These fields should be removed then. Or at least a point in documentation stating "these fields are there for no reason"

[tobya]

There are two issues here, the real bug which is fixed in #2039 is what @cb22 has hightlighted and is an issue for 0.10.11 in that proxy or any other directive will loose its query for request url if the site has a directory attached.

The issue raised by @sirkon is a more general issue about caddy as Scheme and Host have never been available on this code path. That information is available elsewhere in the code. @mholt explains above.

[mholt]

@sirkon Fields can't be "removed" from a struct type. Zero-value fields will remain empty.

As Toby mentioned, the real bug here is the missing query string. Everything else is as expected. See https://golang.org/pkg/net/http#Request - and RFC 2616, Section 5.1.2.

Thanks Toby! And thanks for the report, Denis. We'll finish this up in the PR.