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

net/url: parseQuery() does not check for space in url #39775

Open
TidesOfMemories opened this issue Jun 23, 2020 · 7 comments
Open

net/url: parseQuery() does not check for space in url #39775

TidesOfMemories opened this issue Jun 23, 2020 · 7 comments
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@TidesOfMemories
Copy link

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

$ go version
go version go1.14.2 linux/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/eepgnao/.cache/go-build"
GOENV="/home/eepgnao/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/eepgnao/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/eepgnao/.linuxbrew/Cellar/go/1.14.2_1/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/eepgnao/.linuxbrew/Cellar/go/1.14.2_1/libexec/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc-7"
CXX="g++-7"
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-build885884852=/tmp/go-build -gno-record-gcc-switches"

What did you do?

What did you expect to see?

An err value is returned when the query URL contains space character

What did you see instead?

space character is parsed into the value of the dict

@davecheney
Copy link
Contributor

Please provide a runnable sample to demonstrate what you did. Thank you

@davecheney davecheney added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jun 23, 2020
@TidesOfMemories
Copy link
Author

TidesOfMemories commented Jun 23, 2020

@davecheney

package main

import (
	"net/url"
	"fmt"
)

func main() {
	oriqueryForm0, err0 := url.ParseQuery(`http://192.168.240.36:3000/nnrf-disc/v1/nf-instances?service-names=  &requester-nf-instance-fqdn=seliiu`)
	if err0 == nil {
	    fmt.Printf("service names = %s", oriqueryForm0["service-names"])
	}
}

//The code above should not print anything out because of the space char behind "service-names="

@davecheney
Copy link
Contributor

Thank you for your sample code, why do you believe that the space should terminate the URL? Do you have an example where a browser or client send this URL?

@TidesOfMemories
Copy link
Author

@davecheney
According to RFC 3986, space characters should not be allowed in URLs.
In most cases, browsers will translate space characters to %20 or terminate the request.
In some applications(such as bare metal embedded systems) where clients are not using a browser and there is no validation check for the URL, then the server will not be able to detect the error in the URL if the incoming request URL is used as an argument for parseQuery
I used a simple HTTP2 client & server demo code I found on the internet and verified the behavior. Please let me know if you are interested in seeing the code.

@networkimprov
Copy link

cc @bradfitz

@davecheney
Copy link
Contributor

@pp5311006 i think this is a question of where the url comes from. It is not possible for a URL with a space in it to be transmitted by HTTP. The space could be present if the url was stored in a database or file or read via input. This sounds like an input validation problem, but equally given the problems this could cause maybe ParseQuery could be stricter about the input it accepts.

@TidesOfMemories
Copy link
Author

@davecheney
You are right. I would love to see ParseQuery function having a stricter input validation check.
Currently we are trying to prevent this problem by doing extra validation check, it would be nice to have it in ParseQuery by default.

@ALTree ALTree added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Jun 18, 2021
@seankhliao seankhliao added this to the Unplanned milestone Aug 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

5 participants