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

Port stale simple_keys fix to v2 #543

Merged
merged 2 commits into from Nov 19, 2019
Merged

Port stale simple_keys fix to v2 #543

merged 2 commits into from Nov 19, 2019

Conversation

cjcullen
Copy link

@cjcullen cjcullen commented Nov 7, 2019

Port @niemeyer's e228e37 patch back to v2 to fix the issues highlighted in #537.

This PR includes the tests from #537 (as valid YAML this time!)

cjcullen and others added 2 commits Nov 7, 2019
This should simplify the logic and significantly improve
performance in edge cases as found and reported on go-yaml#537
by CJ Cullen.
@cjcullen
Copy link
Author

@cjcullen cjcullen commented Nov 7, 2019

$ benchcmp old.txt new.txt
benchmark                                old ns/op       new ns/op      delta
Benchmark1000KB100Aliases-6              1091120350      1075070347     -1.47%
Benchmark1000KBDeeplyNestedSlices-6      280989320       49712141       -82.31%
Benchmark1000KBDeeplyNestedMaps-6        273605193       48815122       -82.16%
Benchmark1000KBDeeplyNestedIndents-6     4763691         4166355        -12.54%
Benchmark1000KB1000IndentLines-6         447594119       442198300      -1.21%
Benchmark1KBMaps-6                       608726          573367         -5.81%
Benchmark10KBMaps-6                      5959418         5894779        -1.08%
Benchmark100KBMaps-6                     54540557        52489583       -3.76%
Benchmark1000KBMaps-6                    506866037       490304993      -3.27%
BenchmarkDeepSlice-6                     46990727383     455358199      -99.03%
BenchmarkDeepFlow-6                      43430308616     401182420      -99.08%

benchmark                                old allocs     new allocs     delta
Benchmark1000KB100Aliases-6              5832439        5832445        +0.00%
Benchmark1000KBDeeplyNestedSlices-6      9068           9067           -0.01%
Benchmark1000KBDeeplyNestedMaps-6        9070           9070           +0.00%
Benchmark1000KBDeeplyNestedIndents-6     10083          10082          -0.01%
Benchmark1000KB1000IndentLines-6         4093516        4093516        +0.00%
Benchmark1KBMaps-6                       3126           3126           +0.00%
Benchmark10KBMaps-6                      30780          30780          +0.00%
Benchmark100KBMaps-6                     307269         307269         +0.00%
Benchmark1000KBMaps-6                    3072079        3072079        +0.00%
BenchmarkDeepSlice-6                     2048121        2048121        +0.00%
BenchmarkDeepFlow-6                      1978104        1978105        +0.00%

benchmark                                old bytes     new bytes     delta
Benchmark1000KB100Aliases-6              393119112     393121584     +0.00%
Benchmark1000KBDeeplyNestedSlices-6      4689806       4689598       -0.00%
Benchmark1000KBDeeplyNestedMaps-6        4689918       4689794       -0.00%
Benchmark1000KBDeeplyNestedIndents-6     2969946       2969941       -0.00%
Benchmark1000KB1000IndentLines-6         143718512     143718544     +0.00%
Benchmark1KBMaps-6                       218985        218984        -0.00%
Benchmark10KBMaps-6                      2178157       2178160       +0.00%
Benchmark100KBMaps-6                     22002824      22002821      -0.00%
Benchmark1000KBMaps-6                    220560496     220560528     +0.00%
BenchmarkDeepSlice-6                     120114840     120114840     +0.00%
BenchmarkDeepFlow-6                      115057968     115058112     +0.00%

@niemeyer
Copy link
Contributor

@niemeyer niemeyer commented Nov 19, 2019

Thank you!

@niemeyer niemeyer merged commit 36babc3 into go-yaml:v2 Nov 19, 2019
1 check passed
thaJeztah added a commit to thaJeztah/yaml that referenced this issue Nov 28, 2019
full diff: go-yaml/yaml@v2.2.2...v2.2.7

includes:

- go-yaml/yaml@caeefd8
  addresses CVE-2019-11253 JSON/YAML parsing vulnerable to resource exhaustion attack
- go-yaml/yaml#171 Tighten restrictions on float decoding
- go-yaml/yaml#515 Add large document benchmarks, tune alias heuristic, add max depth limits
- go-yaml/yaml@f90ceb4
  fixes go-yaml/yaml#529 yaml.Unmarshal crashes on "assignment to entry in nil map"
- go-yaml/yaml#543 Port stale simple_keys fix to v2
- go-yaml/yaml@1f64d61
  fixes go-yaml/yaml#548 Invalid simple_keys now cause panics later in decode

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/yaml that referenced this issue Nov 28, 2019
full diff: go-yaml/yaml@v2.2.2...v2.2.7

includes:

- go-yaml/yaml@caeefd8
  addresses CVE-2019-11253 JSON/YAML parsing vulnerable to resource exhaustion attack
- go-yaml/yaml#171 Tighten restrictions on float decoding
- go-yaml/yaml#515 Add large document benchmarks, tune alias heuristic, add max depth limits
- go-yaml/yaml@f90ceb4
  fixes go-yaml/yaml#529 yaml.Unmarshal crashes on "assignment to entry in nil map"
- go-yaml/yaml#543 Port stale simple_keys fix to v2
- go-yaml/yaml@1f64d61
  fixes go-yaml/yaml#548 Invalid simple_keys now cause panics later in decode

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants