simple_oauth icon indicating copy to clipboard operation
simple_oauth copied to clipboard

refactor .parse for robustness

Open notEthan opened this issue 10 years ago • 5 comments

following #13 I have noticed a couple other issues, all of which I've fixed by refactoring .parse.

  • a badly formed Authorization header results in the rather unuseful error NoMethodError: undefined method '[]' for nil:NilClass. I've reworked things to raise a SimpleOauth::ParseError with a relatively informative message.
  • while other characters may be passed in without proper escaping (despite this not being strictly valid per the OAuth spec), commas passed in unescaped break the parsing. I've changed to allow unescaped commas the same as any other character. this necessitated a change to use a StringScanner rather than simply splitting, which I think makes the whole thing more robust.
  • Authorization header params prefixed with oauth_ are treated the same as those without the oauth_ prefix. I check that all params are properly prefixed with oauth_ and silently ignore them if they are not recognized parameters starting with oauth_ ~~raise ParseError if not~~ [changed].

notEthan avatar Mar 12 '14 17:03 notEthan

Coverage Status

Coverage remained the same when pulling 6f545558911c6980e0c4ad1e0c3fa277de9b54e1 on notEthan:parse_error into f60702269d9d6dee322a904ce02645533ddd1a8d on laserlemon:master.

coveralls avatar Mar 12 '14 20:03 coveralls

Coverage Status

Coverage remained the same when pulling 6f545558911c6980e0c4ad1e0c3fa277de9b54e1 on notEthan:parse_error into f60702269d9d6dee322a904ce02645533ddd1a8d on laserlemon:master.

coveralls avatar Mar 12 '14 22:03 coveralls

Coverage Status

Coverage remained the same when pulling cf6d3c4f3dacda05f06f1f85c0119ae3d7a21881 on notEthan:parse_error into a4443c04f5f491435fdc3f676f2b679dccc27adf on laserlemon:master.

coveralls avatar Mar 27 '14 20:03 coveralls

Coverage Status

Coverage remained the same when pulling f73a2b70d2d2e198cb5ab75174394222fe9b13e7 on notEthan:parse_error into a4443c04f5f491435fdc3f676f2b679dccc27adf on laserlemon:master.

coveralls avatar Apr 14 '14 07:04 coveralls

updated to not raise on unrecognized Authorization params, which should be allowed, e.g. 'realm'. instead these are silently ignored/discarded. not sure if that's the best approach; another option would be to return them on string keys, but that has some weird inconsistencies.

not sure anyone is even looking at this, though.

notEthan avatar Apr 14 '14 07:04 notEthan