Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/build/status/statusserver: seemingly polar opposites of contentLength and error message #25246

Closed
odeke-em opened this issue May 3, 2018 · 3 comments
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge
Milestone

Comments

@odeke-em
Copy link
Member

odeke-em commented May 3, 2018

I was just studying the build system and noticed this code
https://github.com/golang/build/blob/8c46052de7fa644a592f162aa0ffeedd959a9701/status/statusserver/statusserver.go#L115-L118
that is
screen shot 2018-05-03 at 4 35 40 pm

Notice that it checks if ContentLength > 0 && ContentLenght <= 1<<20 but then outputs an error request requires explicit Content-Length under 1MB

What's the intention here? Are we rejecting content of:
a) less than 1MB in which case we should fix the error message
b) more than 1MB in which case we should fix the condition from <= to >=

Here is a repro

package main

import (
	"crypto/tls"
	"io/ioutil"
	"net/http"
	"net/http/httptest"
	"net/url"
	"testing"

	"golang.org/x/build/status/statusserver"
)

func TestStatusServer(t *testing.T) {
	h := statusserver.NewHandler()

	req := &http.Request{
		TLS: new(tls.ConnectionState), Method: "POST",
		Header:        map[string][]string{"Content-Type": []string{"application/x-www-form-urlencoded"}},
		URL:           &url.URL{Path: "/status/update"},
		ContentLength: 10,
	}

	rec := httptest.NewRecorder()
	h.ServeHTTP(rec, req)
	res := rec.Result()
	_, err := ioutil.ReadAll(res.Body)
	if err != nil {
		t.Fatalf("Reading body unexpected error %v", err)
	}
	res.Body.Close()
	if g, w := res.StatusCode, http.StatusNoContent; g != w {
		t.Errorf("StatusCode got %d want %d", g, w)
	}
}
@gopherbot gopherbot added this to the Unreleased milestone May 3, 2018
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label May 3, 2018
@odeke-em
Copy link
Member Author

odeke-em commented May 3, 2018

/cc @bradfitz @andybons

@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label May 3, 2018
@bradfitz bradfitz self-assigned this May 4, 2018
@bradfitz
Copy link
Contributor

bradfitz commented May 4, 2018

Thanks, will fix. That code isn't in use yet.

@gopherbot
Copy link

Change https://golang.org/cl/179477 mentions this issue: status, status/statusserver: add README, fix bug

@golang golang locked and limited conversation to collaborators May 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

3 participants