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

cli: upgrade to go1.18.5 #2223

Closed
bassosimone opened this issue Aug 22, 2022 · 0 comments · Fixed by ooni/probe-cli#872
Closed

cli: upgrade to go1.18.5 #2223

bassosimone opened this issue Aug 22, 2022 · 0 comments · Fixed by ooni/probe-cli#872
Assignees
Labels

Comments

@bassosimone
Copy link
Contributor

bassosimone commented Aug 22, 2022

Because we cannot upgrade directly to go1.19 (issue #2211) because of issue #2222, let's update to go1.18.5. This entails ensuring that the oohttp and oocrypto versions we're using are 100% compatible with go1.18.5 (they're currently using go1.19, unfortunately). Here's what changed between go1.18.3 and go1.18.5 in src/net/http:

% git diff go1.18.3...go1.18.5 -- ./src/net/http
diff --git a/src/net/http/header.go b/src/net/http/header.go
index 6487e5025d..6437f2d2c0 100644
--- a/src/net/http/header.go
+++ b/src/net/http/header.go
@@ -103,6 +103,12 @@ func (h Header) Clone() Header {
 	sv := make([]string, nv) // shared backing array for headers' values
 	h2 := make(Header, len(h))
 	for k, vv := range h {
+		if vv == nil {
+			// Preserve nil values. ReverseProxy distinguishes
+			// between nil and zero-length header values.
+			h2[k] = nil
+			continue
+		}
 		n := copy(sv, vv)
 		h2[k] = sv[:n:n]
 		sv = sv[n:]
diff --git a/src/net/http/header_test.go b/src/net/http/header_test.go
index 57d16f51a5..0b13d311ac 100644
--- a/src/net/http/header_test.go
+++ b/src/net/http/header_test.go
@@ -248,6 +248,11 @@ func TestCloneOrMakeHeader(t *testing.T) {
 			in:   Header{"foo": {"bar"}},
 			want: Header{"foo": {"bar"}},
 		},
+		{
+			name: "nil value",
+			in:   Header{"foo": nil},
+			want: Header{"foo": nil},
+		},
 	}
 
 	for _, tt := range tests {
diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index fb18cb2c6f..3f3e1421a9 100644
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -6250,6 +6250,7 @@ func TestUnsupportedTransferEncodingsReturn501(t *testing.T) {
 		"fugazi",
 		"foo-bar",
 		"unknown",
+		"\rchunked",
 	}
 
 	for _, badTE := range unsupportedTEs {
diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go
index 6d51178ee9..de69d835ac 100644
--- a/src/net/http/transfer.go
+++ b/src/net/http/transfer.go
@@ -640,7 +640,7 @@ func (t *transferReader) parseTransferEncoding() error {
 	if len(raw) != 1 {
 		return &unsupportedTEError{fmt.Sprintf("too many transfer encodings: %q", raw)}
 	}
-	if !ascii.EqualFold(textproto.TrimString(raw[0]), "chunked") {
+	if !ascii.EqualFold(raw[0], "chunked") {
 		return &unsupportedTEError{fmt.Sprintf("unsupported transfer encoding: %q", raw[0])}
 	}
 

And here's what changed in src/crypto:

% git diff go1.18.3...go1.18.5 -- ./src/crypto

That is, nothing actually changes.

From this, it stems that we should only update oohttp (which we did in ooni/oohttp#31) and we should not change anything in oocrypto. Yet, we may need some extra changes to support go1.19 as well.

@bassosimone bassosimone self-assigned this Aug 22, 2022
bassosimone added a commit to ooni/oohttp that referenced this issue Aug 22, 2022
Merge go1.18.5 into the stable v0.2.x train

Part of ooni/probe#2223
bassosimone added a commit to ooni/oohttp that referenced this issue Aug 22, 2022
I forgot to change the entry before.

Spotted while working on ooni/probe#2223.
bassosimone added a commit to ooni/oohttp that referenced this issue Aug 22, 2022
bassosimone added a commit to ooni/oohttp that referenced this issue Aug 22, 2022
Part of ooni/probe#2223

I'm reading the diff with respect to main and applying changes
that should also be part of the stable branch.
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 22, 2022
This diff pins OONI to use go1.18.5 and oohttp and oocrypto at
go1.18.5 as the version of go with which we build releases.

The same codebase also works with go1.19 although this configuration
cannot include Psiphon (see ooni/probe#2222).

Closes ooni/probe#2223.
bassosimone added a commit to ooni/oocrypto that referenced this issue Aug 22, 2022
This diff backports code to correctly handle darwin/arm64 from
`main` (see #7).

While there, this diff aims at reducing unnecessary differences with
the main branch, by removing code we also removed in there.

The reference issues are:

1. ooni/probe#2122

2. ooni/probe#2223
bassosimone added a commit to ooni/oocrypto that referenced this issue Aug 22, 2022
This diff backports code to correctly handle darwin/arm64 from
`main` (see #7).

While there, this diff aims at reducing unnecessary differences with
the main branch, by removing code we also removed in there.

The reference issues are:

1. ooni/probe#2122

2. ooni/probe#2223
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 22, 2022
* chore: use {go,oohttp,oocrypto} v1.18.5

This diff pins OONI to use go1.18.5 and oohttp and oocrypto at
go1.18.5 as the version of go with which we build releases.

The same codebase also works with go1.19 although this configuration
cannot include Psiphon (see ooni/probe#2222).

Closes ooni/probe#2223.

* fix: use oocrypto@v0.1.1

This ensures we keep ooni/probe#2122 fixed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant