Commit 36c20d1f authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub

Merge pull request #38525 from juanvallejo/jvallejo/fix-panic-on-invalid-json-syntax

Automatic merge from submit-queue (batch tested with PRs 38525, 38977) Prevent json decoder panic on invalid input Related downstream issue: https://github.com/openshift/origin/issues/12132 ``` # Can be replicated on kubectl with: $ cat panic.json { "kind": "Pod", "apiVersion": "v1", "metadata": { "name": "", "labels": { "name": "" }, "generateName": "", "namespace": "", "annotations": [] }, "spec": {} }, $ kubectl create -f panic.json --validate=false ``` **Release note**: ```release-note release-note-none ``` This patch handles cases where `ioutil.ReadAll` will return a single character output on an invalid json input, causing the `Decode` method to panic when it tries to calculate the line number for the syntax error. The example below would cause a panic due to the trailing comma at the end: ``` { "kind": "Pod", "apiVersion": "v1", "metadata": { "name": "", "labels": { "name": "" }, "generateName": "", "namespace": "", "annotations": [] }, "spec": {} }, ``` @kubernetes/cli-review @fabianofranz
parents def80227 07ca2198
...@@ -194,7 +194,7 @@ func (s *Serializer) RecognizesData(peek io.Reader) (ok, unknown bool, err error ...@@ -194,7 +194,7 @@ func (s *Serializer) RecognizesData(peek io.Reader) (ok, unknown bool, err error
// we could potentially look for '---' // we could potentially look for '---'
return false, true, nil return false, true, nil
} }
_, ok = utilyaml.GuessJSONStream(peek, 2048) _, _, ok = utilyaml.GuessJSONStream(peek, 2048)
return ok, false, nil return ok, false, nil
} }
......
...@@ -181,6 +181,7 @@ type YAMLOrJSONDecoder struct { ...@@ -181,6 +181,7 @@ type YAMLOrJSONDecoder struct {
bufferSize int bufferSize int
decoder decoder decoder decoder
rawData []byte
} }
// NewYAMLOrJSONDecoder returns a decoder that will process YAML documents // NewYAMLOrJSONDecoder returns a decoder that will process YAML documents
...@@ -198,10 +199,11 @@ func NewYAMLOrJSONDecoder(r io.Reader, bufferSize int) *YAMLOrJSONDecoder { ...@@ -198,10 +199,11 @@ func NewYAMLOrJSONDecoder(r io.Reader, bufferSize int) *YAMLOrJSONDecoder {
// provide object, or returns an error. // provide object, or returns an error.
func (d *YAMLOrJSONDecoder) Decode(into interface{}) error { func (d *YAMLOrJSONDecoder) Decode(into interface{}) error {
if d.decoder == nil { if d.decoder == nil {
buffer, isJSON := GuessJSONStream(d.r, d.bufferSize) buffer, origData, isJSON := GuessJSONStream(d.r, d.bufferSize)
if isJSON { if isJSON {
glog.V(4).Infof("decoding stream as JSON") glog.V(4).Infof("decoding stream as JSON")
d.decoder = json.NewDecoder(buffer) d.decoder = json.NewDecoder(buffer)
d.rawData = origData
} else { } else {
glog.V(4).Infof("decoding stream as YAML") glog.V(4).Infof("decoding stream as YAML")
d.decoder = NewYAMLToJSONDecoder(buffer) d.decoder = NewYAMLToJSONDecoder(buffer)
...@@ -215,6 +217,13 @@ func (d *YAMLOrJSONDecoder) Decode(into interface{}) error { ...@@ -215,6 +217,13 @@ func (d *YAMLOrJSONDecoder) Decode(into interface{}) error {
glog.V(4).Infof("reading stream failed: %v", readErr) glog.V(4).Infof("reading stream failed: %v", readErr)
} }
js := string(data) js := string(data)
// if contents from io.Reader are not complete,
// use the original raw data to prevent panic
if int64(len(js)) <= syntax.Offset {
js = string(d.rawData)
}
start := strings.LastIndex(js[:syntax.Offset], "\n") + 1 start := strings.LastIndex(js[:syntax.Offset], "\n") + 1
line := strings.Count(js[:start], "\n") line := strings.Count(js[:start], "\n")
return fmt.Errorf("json: line %d: %s", line, syntax.Error()) return fmt.Errorf("json: line %d: %s", line, syntax.Error())
...@@ -296,10 +305,10 @@ func (r *LineReader) Read() ([]byte, error) { ...@@ -296,10 +305,10 @@ func (r *LineReader) Read() ([]byte, error) {
// GuessJSONStream scans the provided reader up to size, looking // GuessJSONStream scans the provided reader up to size, looking
// for an open brace indicating this is JSON. It will return the // for an open brace indicating this is JSON. It will return the
// bufio.Reader it creates for the consumer. // bufio.Reader it creates for the consumer.
func GuessJSONStream(r io.Reader, size int) (io.Reader, bool) { func GuessJSONStream(r io.Reader, size int) (io.Reader, []byte, bool) {
buffer := bufio.NewReaderSize(r, size) buffer := bufio.NewReaderSize(r, size)
b, _ := buffer.Peek(size) b, _ := buffer.Peek(size)
return buffer, hasJSONPrefix(b) return buffer, b, hasJSONPrefix(b)
} }
var jsonPrefix = []byte("{") var jsonPrefix = []byte("{")
......
...@@ -65,7 +65,7 @@ func TestSplitYAMLDocument(t *testing.T) { ...@@ -65,7 +65,7 @@ func TestSplitYAMLDocument(t *testing.T) {
} }
func TestGuessJSON(t *testing.T) { func TestGuessJSON(t *testing.T) {
if r, isJSON := GuessJSONStream(bytes.NewReader([]byte(" \n{}")), 100); !isJSON { if r, _, isJSON := GuessJSONStream(bytes.NewReader([]byte(" \n{}")), 100); !isJSON {
t.Fatalf("expected stream to be JSON") t.Fatalf("expected stream to be JSON")
} else { } else {
b := make([]byte, 30) b := make([]byte, 30)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment