Skip to content

Instantly share code, notes, and snippets.

@benmmurphy
Created February 4, 2013 10:45
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save benmmurphy/4706099 to your computer and use it in GitHub Desktop.
Save benmmurphy/4706099 to your computer and use it in GitHub Desktop.
csv object loading
irb(main):004:0> CSV.dump([Object.new])
=> "class,Object\n\n\n"
irb(main):005:0> CSV.load(CSV.dump([Object.new]))
=> [#<Object:0x00000100ae90d8>]
@mboeh
Copy link

mboeh commented Feb 4, 2013

CSV.load "class,Object\neval\nputs 'hello'\n"

I wonder if anyone actually uses the dump/load protocol on CSV, though. I wasn't aware of it.

ETA: searching 'CSV.load' on GitHub has two results, neither of which are relevant.

@amalagaura
Copy link

This is very serious. One good thing is that CSV.open does not seem to be affected.

@jredville
Copy link

Sadly, I bet the people that use it do so because they don't know better. I always get lost on the CSV api :|

@dball
Copy link

dball commented Feb 5, 2013

FWIW, @JEG3 is removing this feature. Note I couldn't find any instances of it in the wild the other day when I went looking for them to satisfy my own curiosity.

@nickmartini
Copy link

Welp.

@JEG2
Copy link

JEG2 commented Feb 5, 2013

I do not agree that this was a super serious issue, but it is now resolved.

I removed the feature because I have never seen anyone use it, it's trivial to reimplement if you need it, and I saw no value in spending energy to lock it down. This is nothing people should waste Gist comments worrying about. :)

Just to be clear, we have always been discussing an experimental side feature. Normal CSV reading/writing operations were not and are not vulnerable.

I am sad to read that CSV's API confuses people. It is pretty well documented, in my opinion. If you look into the CSV object in the documentation it shows the common usage right at the top. I'm happy to take patches that clarifies any confusions though.

Even if the API was hard to understand, I still doubt that anyone was accidentally using load(). It required a special format in the first two lines, so it probably would have just died on almost any content not produced by dump().

This is my appraisal of the situation, for what it's worth.

@blowmage
Copy link

blowmage commented Feb 5, 2013

@JEG2 ❤️

@zzak
Copy link

zzak commented Feb 5, 2013

I will also help review and commit any patches for CSV's documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment