ruby / ruby/psych

Sexagesimal scalars are mis-scanned

Open Beginner friendly
#814 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Ruby
Stars
597
Forks
223
Avg merge
11h 23m
Merged PRs (30d)
3

Description

Summary

Psych::ScalarScanner resolves YAML 1.1 base-60 integers with a positional weight that assumes exactly three components, and applies the leading - to only the first component. Two independent bugs result: 1. Any two-component sexagesimal (12:30) is 60× too large. 2. Any negative sexagesimal with a non-zero trailing component (-12:30:00) has the wrong magnitude, independent of (1). A consequence of (1) is that 12:30 and 12:30:00 two distinct documents load to the same Ruby value.

Environment
  • ruby 3.4.10
  • psych 5.2.2 (bundled)
Reproduction
ruby require 'yaml' Psych.safe_load('a: 12:30')['a']      # 45000, expected 750 
Psych.safe_load('a: 1:30')['a']       #5400, expected 90 
Psych.safe_load('a: 100:00')['a']     #360000, expected 6000 
Psych.safe_load('a: 12:30.5')['a']    #45030.0, expected 750.5 
Psych.safe_load('a: -12:30')['a']     #-41400, expected -750 
Psych.safe_load('a: -12:30:00')['a']  #-41400, expected -45000    
Expected results

Per the YAML 1.1 int type, the sexagesimal form is [-+]?[1-9][0-9_]*(:[0-5]?[0-9])+, and the spec's canonical example fixes the weighting: sexagesimal: 190:20:30 -> 685230 i.e. 190*60^2 + 20*60^1 + 30*60^0. Components are weighted from the right, so the rightmost component always has weight 60^0. Therefore 12:30 = 12*60 + 30 = 750. Psych parses 190:20:30 correctly (685230), which confirms right-alignment is intended the three-component case is right only because it is the case the code hard-codes.

Root cause

lib/psych/scalar_scanner.rb:
ruby elsif string.match?(/^[-+]?[0-9][0-9_]*(:[0-5]?[0-9]){1,2}$/) i = 0 string.split(':').each_with_index do |n,e| i += (n.to_i * 60 ** (e - 2).abs) end i
60 ** (e - 2).abs weights by distance from index 2, i.e. it assumes the string always has three components. With two components it yields 60^2 and 60^1 instead of 60^1 and 60^0. n.to_i also carries the sign only on the first element, so subsequent components are added as positive magnitudes: -12:30:00 becomes -43200 + 1800 + 0. Two smaller spec deviations in the same regex: {1,2} caps the form at three components where the spec allows (...)+, and the leading component is [0-9] where the spec says [1-9] (so 0:30 is resolved as an integer when it should stay a String).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in lib/psych/scalar_scanner.rb at the sexagesimal branch and run the listed Psych.safe_load reproductions. Check the YAML 1.1 integer specification linked in the issue for right-aligned base-60 weighting, sign handling, and accepted component counts. Done means the examples produce the expected Ruby values and invalid forms such as 0:30 remain strings.

Written by the indexing model from the issue text.

Assessment

Tech stack
ruby
Domain
tooling
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.