Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Ruby 1.8.6 compatibility? #7

Closed
nzoschke opened this Issue · 5 comments

2 participants

@nzoschke

I have been using OkJson in a legacy ruby environment (Heroku aspen stack). In general it works, but occasionally throws exceptions. I ran the test suite and sure enough it doesn't pass. Would you accept a patch for 1.8.6 compatibility?

$ ruby -v
ruby 1.8.6 (2008-08-11 patchlevel 287) [x86_64-linux]

$ ./test 
t/invalid10.json
t/invalid11.json
t/invalid12.json
t/invalid13.json
t/invalid14.json
t/invalid15.json
t/invalid16.json
t/invalid17.json
t/invalid19.json
t/invalid1.json
t/invalid20.json
t/invalid21.json
t/invalid22.json
t/invalid23.json
t/invalid24.json
t/invalid25.json
t/invalid26.json
t/invalid27.json
t/invalid28.json
t/invalid2.json
t/invalid3.json
t/invalid4.json
t/invalid5.json
t/invalid6.json
t/invalid7.json
t/invalid8.json
t/invalid9.json
t/invalid-key.json
t/invalid-uescape.json
t/valid-0.json
t/valid-1.json
t/valid-alnum.json
t/valid-array-deep.json
t/valid-array-empty.json
t/valid-array-single.json
t/valid-backslash.json
t/valid-bignum.json
t/valid-comment2.json
t/valid-comment.json
t/valid-compact.json
t/valid-escape.json
t/valid-false.json
t/valid-frac-e-minus.json
t/valid-frac-E-plus.json
t/valid-frac.json
t/valid-hex.json
./okjson.rb:386:in `nibble': undefined method `ord' for 48:Fixnum (NoMethodError)
    from ./okjson.rb:367:in `hexdec4'
    from ./okjson.rb:295:in `unquote'
    from ./okjson.rb:248:in `strtok'
    from ./okjson.rb:212:in `tok'
    from ./okjson.rb:175:in `lex'
    from ./okjson.rb:43:in `decode'
    from -e:1
@nzoschke

A simple 1.8.6 ~ 1.9 forward compatibility monkeypatch gets the tests passing, as a starting point.

diff --git a/okjson.rb b/okjson.rb
index 79af516..c8ce76b 100644
--- a/okjson.rb
+++ b/okjson.rb
@@ -22,6 +22,12 @@

 # See https://github.com/kr/okjson for updates.

+unless Fixnum.methods.include? :ord
+  class Fixnum
+    def ord; self; end
+  end
+end
+
 require 'stringio'

 # Some parts adapted from
@kr
Owner
kr commented

Absolutely I'd accept a patch. Some time ago I tried
to make it compatible with 1.8.6, but it seemed hard.
Now I can't remember why.

Is there a way to do it without monkey patching system
classes? Is it possible to write an ord function that
always does the right thing? Then instead of c.ord
we could write ord(c).

@nzoschke

It's tricky... Here's another patch that gets tests passing on 1.8.6 and 1.9.3.

By trial and error, I changed the string slice syntax to the str[regexp, fixnum] => new_str form. Perhaps this should be more carefully applied everywhere.

I also took out the incompatible ?0 string syntax.

Finally, I'm using unpack instead of ord, but since this doesn't have the same UTF-8 handling properties I didn't replace every ord call. Do you know how I could add this assertion to the unpack value?

I can work on this more, just wanted to get some feedback that I'm on the right track.

diff --git a/okjson.rb b/okjson.rb
index 79af516..5b8ac4d 100644
--- a/okjson.rb
+++ b/okjson.rb
@@ -364,7 +364,7 @@ module OkJson
     if s.length != 4
       raise Error, 'short'
     end
-    (nibble(s[0])<<12) | (nibble(s[1])<<8) | (nibble(s[2])<<4) | nibble(s[3])
+    (nibble(s[0,1])<<12) | (nibble(s[1,1])<<8) | (nibble(s[2,1])<<4) | nibble(s[3,1])
   end


@@ -383,14 +383,19 @@ module OkJson

   def nibble(c)
     case true
-    when ?0 <= c && c <= ?9 then c.ord - ?0.ord
-    when ?a <= c && c <= ?z then c.ord - ?a.ord + 10
-    when ?A <= c && c <= ?Z then c.ord - ?A.ord + 10
+    when "0" <= c && c <= "9" then ord(c) - ord("0")
+    when "a" <= c && c <= "z" then ord(c) - ord("a") + 10
+    when "A" <= c && c <= "Z" then ord(c) - ord("A") + 10
     else
       raise Error, "invalid hex code #{c}"
     end
   end

+  def ord(c)
+    # TODO: raise an error if c is invalid UTF-8
+    c.unpack("C*")[0]
+  end
+

   # Encodes x into a json text. It may contain only
   # Array, Hash, String, Numeric, true, false, nil.
@@ -503,7 +508,7 @@ module OkJson
     n = s.length - i
     raise Utf8Error if n < 1

-    c0 = s[i].ord
+    c0 = ord(s[i,1])

     # 1-byte, 7-bit sequence?
     if c0 < Utagx
@@ -514,7 +519,7 @@ module OkJson
     raise Utf8Error if c0 < Utag2 # unexpected continuation byte?

     raise Utf8Error if n < 2 # need continuation byte
-    c1 = s[i+1].ord
+    c1 = ord(s[i+1,1])
     raise Utf8Error if c1 < Utagx || Utag2 <= c1

     # 2-byte, 11-bit sequence?
@@ -528,7 +533,7 @@ module OkJson
     # need second continuation byte
     raise Utf8Error if n < 3

-    c2 = s[i+2].ord
+    c2 = ord(s[i+2,1])
     raise Utf8Error if c2 < Utagx || Utag2 <= c2

     # 3-byte, 16-bit sequence?
@@ -543,7 +548,7 @@ module OkJson

     # need third continuation byte
     raise Utf8Error if n < 4
-    c3 = s[i+3].ord
+    c3 = ord(s[i+3,1])
     raise Utf8Error if c3 < Utagx || Utag2 <= c3

     # 4-byte, 21-bit sequence?
@nzoschke

FWIW, I applied the Fixnum monkeypatch to my 1.8.6 app outside of okjson and am having no problem with that in production.

@kr
Owner
kr commented

This is getting pretty stale, and I think demand for 1.8.6 is
decreasing rapidly. Anyone, feel free to reopen if this becomes
more of an issue.

@kr kr closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.