net/url: regression after #22907 fixed #27302

ayanamist · 2018-08-28T12:02:20Z

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (`go version`)?

go version devel +76c4587 Tue Aug 28 09:26:45 2018 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (`go env`)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/tianyang.yty/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/tianyang.yty/go"
GOPROXY=""
GORACE=""
GOROOT="/home/tianyang.yty/gobuild/go"
GOTMPDIR=""
GOTOOLDIR="/home/tianyang.yty/gobuild/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build461801575=/tmp/go-build -gno-record-gcc-switches"

What did you do?

https://play.golang.org/p/bScfUelhUmE

What did you expect to see?

2009/11/10 23:00:00 RawQuery array[]=first&array[]=second
2009/11/10 23:00:00 url.Values{"array[]":[]string{"first", "second"}}

What did you see instead?

2018/08/28 19:57:03 RawQuery array%5B%5D%3Dfirst%26array%5B%5D%3Dsecond
2018/08/28 19:57:03 url.Values{"array[]=first&array[]=second":[]string{""}}

Many javascript framework, like jquery, will append [] to param keys, in current and previous stable release, this works fine, but after 1040626 fix #22907 pushed to master, this backward compatibility has been broken.

The text was updated successfully, but these errors were encountered:

thinkerou · 2018-08-28T14:32:35Z

gin web framework also have same question. thanks!

appleboy · 2018-08-29T00:47:13Z

See the example code and test result from travis

func TestQuery(t *testing.T) {
	u, err := url.Parse("http://bing.com/search?q=dotnet")
	if err != nil {
		t.Fatal(err)
	}

	if u.RawQuery != "q=dotnet" {
		t.Error("RawQuery error")
	}

	u, err = url.Parse("http://bing.com/search?k=v&id=main&id=omit&array[]=first&array[]=second&ids[i]=111&ids[j]=3.14")
	if err != nil {
		t.Fatal(err)
	}

	if u.RawQuery != "k=v&id=main&id=omit&array[]=first&array[]=second&ids[i]=111&ids[j]=3.14" {
		log.Println(u.RawQuery)
		t.Error("RawQuery error")
	}

	log.Println(u.Query())
}

agnivade · 2018-08-29T06:51:31Z

Well, according to RFC 3986 https://tools.ietf.org/html/rfc3986#section-3.4, a fragment can only contain pchar, which allows sub-delims, not gen-delims. So, as I understand, a [] is invalid and should be escaped.

@meirf

ayanamist · 2018-08-29T07:48:21Z

@agnivade Although rfc does not include gen-delims, however real-world clients often accept this.
e.g. Chrome, will treat [] as valid chars, you can see "Query String Parameters" in below screenshot.

rsc · 2018-09-26T17:54:31Z

It seems like we should roll back that CL. This has to keep working (with UTF8 value).

package main

import (
	"log"
	"net/url"
)

func main() {
	u, err := url.Parse("http://bing.com/search?utf8=☺&param=true")
	if err != nil {
		log.Fatal(err)
	}
	
	log.Println("RawQuery", u.RawQuery)

	log.Printf("%#v", u.Query())
}

gopherbot · 2018-09-26T17:55:11Z

Change https://golang.org/cl/137716 mentions this issue: Revert "net/url: escape URL.RawQuery on Parse if it contains invalid characters"

shuLhan · 2018-09-26T22:42:05Z

@rsc Thank you.

golang team revert the net/url issue: golang/go#27302

gopherbot · 2019-01-23T19:12:23Z

Change https://golang.org/cl/159157 mentions this issue: net/url, net/http: reject control characters in URLs

This is a more conservative version of the reverted CL 99135 (which was reverted in CL 137716) The net/url part rejects URLs with ASCII CTLs from being parsed and the net/http part rejects writing them if a bogus url.URL is constructed otherwise. Updates #27302 Updates #22907 Change-Id: I09a2212eb74c63db575223277aec363c55421ed8 Reviewed-on: https://go-review.googlesource.com/c/159157 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org>

gopherbot · 2019-01-24T22:55:16Z

Change https://golang.org/cl/159478 mentions this issue: [release-branch.go1.10] net/url, net/http: reject control characters in URLs

gopherbot · 2019-01-29T17:25:25Z

Change https://golang.org/cl/160178 mentions this issue: net/url, net/http: relax CTL-in-URL validation to only ASCII CTLs

CL 159157 was doing UTF-8 decoding of URLs. URLs aren't really UTF-8, even if sometimes they are in some contexts. Instead, only reject ASCII CTLs. Updates #27302 Updates #22907 Change-Id: Ibd64efa5d3a93263d175aadf1c9f87deb4670c62 Reviewed-on: https://go-review.googlesource.com/c/160178 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>

gopherbot · 2019-01-31T20:25:35Z

Change https://golang.org/cl/160678 mentions this issue: [release-branch.go1.10] net/http, net/url: reject control characters in URLs

gopherbot · 2019-02-01T15:28:14Z

Change https://golang.org/cl/160798 mentions this issue: [release-branch.go1.11] net/http, net/url: reject control characters in URLs

…in URLs Cherry pick of combined CL 159157 + CL 160178. Fixes #29923 Updates #27302 Updates #22907 Change-Id: I6de92c14284595a58321a4b4d53229285979b872 Reviewed-on: https://go-review.googlesource.com/c/160798 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>

…in URLs Cherry pick of combined CL 159157 + CL 160178. Fixes #29922 Updates #27302 Updates #22907 Change-Id: I6de92c14284595a58321a4b4d53229285979b872 Reviewed-on: https://go-review.googlesource.com/c/160678 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>

gopherbot · 2019-02-15T23:43:38Z

Change https://golang.org/cl/162960 mentions this issue: doc/go1.12: document net/url.Parse now rejecting ASCII CTLs

gopherbot · 2019-02-15T23:54:42Z

Change https://golang.org/cl/162826 mentions this issue: [release-branch.go1.12] doc/go1.12: document net/url.Parse now rejecting ASCII CTLs

Updates #27302 Updates #22907 Change-Id: Iac6957f3517265dfb9c662efb7af31192e3bfd6c Reviewed-on: https://go-review.googlesource.com/c/162960 Reviewed-by: Ian Lance Taylor <iant@golang.org>

…ing ASCII CTLs Updates #27302 Updates #22907 Change-Id: Iac6957f3517265dfb9c662efb7af31192e3bfd6c Reviewed-on: https://go-review.googlesource.com/c/162960 Reviewed-by: Ian Lance Taylor <iant@golang.org> (cherry picked from commit ef454fd) Reviewed-on: https://go-review.googlesource.com/c/162826

CL 159157 was doing UTF-8 decoding of URLs. URLs aren't really UTF-8, even if sometimes they are in some contexts. Instead, only reject ASCII CTLs. Updates golang#27302 Updates golang#22907 Change-Id: Ibd64efa5d3a93263d175aadf1c9f87deb4670c62 Reviewed-on: https://go-review.googlesource.com/c/160178 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>

Updates golang#27302 Updates golang#22907 Change-Id: Iac6957f3517265dfb9c662efb7af31192e3bfd6c Reviewed-on: https://go-review.googlesource.com/c/162960 Reviewed-by: Ian Lance Taylor <iant@golang.org>

CL 159157 was doing UTF-8 decoding of URLs. URLs aren't really UTF-8, even if sometimes they are in some contexts. Instead, only reject ASCII CTLs. Updates golang#27302 Updates golang#22907 Change-Id: Ibd64efa5d3a93263d175aadf1c9f87deb4670c62 Reviewed-on: https://go-review.googlesource.com/c/160178 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>

Updates golang#27302 Updates golang#22907 Change-Id: Iac6957f3517265dfb9c662efb7af31192e3bfd6c Reviewed-on: https://go-review.googlesource.com/c/162960 Reviewed-by: Ian Lance Taylor <iant@golang.org>

appleboy mentioned this issue Aug 28, 2018

fix go master branch(1.11+) error about Query and Form gin-gonic/gin#1510

Closed

agnivade added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 29, 2018

agnivade added this to the Go1.12 milestone Aug 29, 2018

gopherbot closed this as completed in b50210f Sep 26, 2018

appleboy added a commit to gin-gonic/gin that referenced this issue Sep 27, 2018

remove allow fail flag

cd69e07

golang team revert the net/url issue: golang/go#27302

appleboy added a commit to gin-gonic/gin that referenced this issue Sep 27, 2018

remove allow fail flag (#1573)

91a4459

golang team revert the net/url issue: golang/go#27302

justinfx pushed a commit to justinfx/gin that referenced this issue Nov 3, 2018

remove allow fail flag (gin-gonic#1573)

e4e4270

golang team revert the net/url issue: golang/go#27302

agnivade mentioned this issue Dec 23, 2018

net/url: PathEscape treatment of reserved characters is inconsistent #27559

Open

dmitshur mentioned this issue Jan 24, 2019

[release-branch.go1.10] net/url, net/http: reject control characters in URLs #29926

Closed

bradfitz mentioned this issue Mar 18, 2019

net/url: Malformed query code breaks backwards compatibility #30903

Closed

tirkarthi mentioned this issue Apr 10, 2019

bpo-30458: Disallow control chars in http URLs. python/cpython#12755

Merged

golang locked and limited conversation to collaborators Feb 15, 2020

gopherbot added the FrozenDueToAge label Feb 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

net/url: regression after #22907 fixed #27302

net/url: regression after #22907 fixed #27302

ayanamist commented Aug 28, 2018

thinkerou commented Aug 28, 2018

appleboy commented Aug 29, 2018 •

edited

agnivade commented Aug 29, 2018

ayanamist commented Aug 29, 2018

rsc commented Sep 26, 2018

gopherbot commented Sep 26, 2018

shuLhan commented Sep 26, 2018

gopherbot commented Jan 23, 2019

gopherbot commented Jan 24, 2019

gopherbot commented Jan 29, 2019

gopherbot commented Jan 31, 2019

gopherbot commented Feb 1, 2019

gopherbot commented Feb 15, 2019

gopherbot commented Feb 15, 2019

net/url: regression after #22907 fixed #27302

net/url: regression after #22907 fixed #27302

Comments

ayanamist commented Aug 28, 2018

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

What did you expect to see?

What did you see instead?

thinkerou commented Aug 28, 2018

appleboy commented Aug 29, 2018 • edited

agnivade commented Aug 29, 2018

ayanamist commented Aug 29, 2018

rsc commented Sep 26, 2018

gopherbot commented Sep 26, 2018

shuLhan commented Sep 26, 2018

gopherbot commented Jan 23, 2019

gopherbot commented Jan 24, 2019

gopherbot commented Jan 29, 2019

gopherbot commented Jan 31, 2019

gopherbot commented Feb 1, 2019

gopherbot commented Feb 15, 2019

gopherbot commented Feb 15, 2019

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?

appleboy commented Aug 29, 2018 •

edited