sideshowcoder / canned

Server to respond with fake API responses, by using a directory of files for finding out what to say!
213 stars 46 forks source link

Apostrophes in JSON response break the comment parser // !params etc #58

Closed simonprickett closed 9 years ago

simonprickett commented 9 years ago

I've noticed that if I use the:

// !params: { "name": "value"}

notation to use a single JSON file to return different responses according to URL parameter value, this will break if the JSON response contains an apostrophe (and maybe some other "special" characters).

Here's an example showing that using the parameter in the filename works, but using the special comment notation does not work.

_test?param=a.get.json

File contains:

{
    "value": "This is value a's response."
}

http://localhost:3000/demo/test?param=a

{
value: "This is value a's response."
}

Works as expected.

_test?param=b.get.json

File contains:

{
    "value": "This is value b's response."
}

http://localhost:3000/demo/test?param=b

{
value: "This is value b's response."
}

Works as expected.

test2.get.json

File contains:

//! params: {"param": "a"}
{
    "value": "This is value a response."
}
//! params: {"param": "b"}
{
    "value": "This is value b response."
}

http://localhost:3000/demo/test2?param=a

{
value: "This is value a response."
}

Works as expected but note I removed the apostrophe from the file.

http://localhost:3000/demo/test2?param=b

{
value: "This is value b response."
}

Works as expected but note I removed the apostrophe from the file.

_test3.get.json

File contains:

//! params: {"param": "a"}
{
    "value": "This is value a's response."
}
//! params: {"param": "b"}
{
    "value": "This is value b's response."
}

http://localhost:3000/demo/test3?param=a

Internal Server error invalid input file

Logs:

request: get /demo/test3?param=aproblem sanatizing content for _test3.get.json SyntaxError: Unexpected end of input served via: ./demo/_test3.get.json

Broken because of ' in response?

http://localhost:3000/demo/test3?param=b

Internal Server error invalid input file

Logs:

request: get /demo/test3?param=bproblem sanatizing content for _test3.get.json SyntaxError: Unexpected end of input served via: ./demo/_test3.get.json

Broken because of ' in response?

I am using node v0.12.0 on Mac OS X 10.10.2. I think this is a problem in the JSON "comment parser", happy to take a look at addressing this, just wanted to see if this is a problem or something in my environment.

simonprickett commented 9 years ago

I added some debug and it looks like it doesn't parse out the complete response and stops at the ':

starting canned on port 3000 for /Users/simon/projects/cannedapi
request: get /demo/test3/test3?param=a//! params: {"param": "a"}
{
    "value": "This is value a
problem sanatizing content for _test3.get.json SyntaxError: Unexpected end of input served via: ./demo/test3/_test3.get.json
simonprickett commented 9 years ago

I think the problem's in "getEachResponse" but it's a bit late night to debug regex will continue tomorrow!

sideshowcoder commented 9 years ago

Thanks for the long explanation, I will try to help as much as I can an lock into it asap but I feat it might take a while so any help you can provide is greatly appriciated!

kevinschumacher commented 9 years ago

Running into this as well, but on my end it's not apostrophes (there are none in my responses, though they are beasts). Any movement on this? If not I can probably try to take a look. Just curious, @simonprickett what has you thinking the issue is in getEachResponse?

sideshowcoder commented 9 years ago

@kevinschumacher I haven't had time to look into this, yet feel free to send some fix along.

kevinschumacher commented 9 years ago

OK, definitely an issue in Canned.prototype.getEachResponse()

Is there any particular reason that regex is being used to break the responses? I think it would probably be less error prone if we just break responses up every time you see "//!" starting a line. Any thoughts on that? I will try the fix and PR shortly

simonprickett commented 9 years ago

Sorry I've been kind of occupied but do mean to circe back -- I was thinking replacing the regex with something to parse out comments on lines starting with //! would lead to more transparent code and that that was likely where the issue was.

kevinschumacher commented 9 years ago

Totally agree. I opened PR with a hacked in fix that shrinks the issue, but the better fix is (probably) to split responses where lines begin //!

sideshowcoder commented 9 years ago

I agree with @simonprickett here and would say that splitting at //! is the more solid route to go, see comment on #59 Thanks so much for your work both of you!