alfred icon indicating copy to clipboard operation
alfred copied to clipboard

Trailing comma in JSON breaks the whole thing

Open tomassasovsky opened this issue 2 years ago • 10 comments

the body I'm sending:

{
    "email": "[email protected]",
}

the error I'm getting:

image

tomassasovsky avatar Mar 28 '22 23:03 tomassasovsky

Ok, so at the end of the day, its invalid JSON. Darts JSON decoder falls over: Screen Shot 2022-03-29 at 10 20 29 am

and even the javascript JSON decoder will throw if you put a trailing comma:

Screen Shot 2022-03-29 at 10 23 31 am

So I don't think it should handle it gracefully. However - it should throw an error that its an invalid body, as opposed to header already sent. So I will do some work on throwing a better error but will leave the invalid JSON alone.

(Edit: the javascript JSON example wasn't valid)

rknell avatar Mar 29 '22 00:03 rknell

I understand, the thing is that it takes the app down. I've tried adding the onInternalError function but still.

tomassasovsky avatar Mar 29 '22 00:03 tomassasovsky

we definitely don't want to crash the server!

rknell avatar Mar 29 '22 00:03 rknell

I just went to have a look at it and the most recent test is there to handle invalid JSON input and return a 400 (thereby not crashing the server) what version of Alfred are you using?

rknell avatar Mar 29 '22 00:03 rknell

using alfred: ^0.1.5+3

tomassasovsky avatar Mar 29 '22 00:03 tomassasovsky

That release was literally the one that added the test for dodgy JSON so it should be working.

It might be that the published version doesn't include the code - ie I made a mistake. Do you mind adding a dependency for the master branch and seeing if that resolves it? If it does I will release a new version.

alfred:
    git: https://github.com/rknell/alfred.git

rknell avatar Mar 29 '22 01:03 rknell

tried that, I'm getting a pretty nasty error: image

tomassasovsky avatar Mar 29 '22 01:03 tomassasovsky

Tried running it directly with dart, but still.. image

tomassasovsky avatar Mar 29 '22 01:03 tomassasovsky

Ahhhh....

So I think its being caused by you using a try catch block and then trying to return something.

I can reproduce it with this:

  test('it handles invalid json input with a trailing comma', () async {
    app.post('/test', (req, res) async {
      try {
        await req.body;
      } catch (e) {
        throw AlfredException(500, {'test': 'response'});
      }
    });
    final response = await http.post(Uri.parse('http://localhost:$port/test'),
        body: '{ "email": "[email protected]",}',
        headers: {'Content-Type': 'application/json'});
    expect(response.statusCode, 400);
  });

I'll see if there is a way not to crash the server when you have already sent a header.

rknell avatar Mar 29 '22 02:03 rknell

Thank you for your help, you have shared very useful information.

I would like to make one addition. If we add double quotes to the key name when submitting Form-Data and we use AlfredException to catch the error, alfred crashes.

Postman:

curl --location --request DELETE '127.0.0.1:3000/testnew'
--form '"fooo"="123"'

Key name "foo" with quotes.

Endpoint Codes:

server.post("/testnew", (req, res) async { try { var testbody = await req.body; return testbody.toString(); } catch (e) { throw AlfredException(401, "Error"); } });

Crash Errors:

Unhandled exception: Bad state: Header already sent #0 _HttpResponse.statusCode= (dart:_http/http_impl.dart:1190:35) #1 Alfred._incomingRequest package:alfred/src/alfred.dart:364 #2 _QueuedFuture.execute package:queue/src/dart_queue_base.dart:26

seceba avatar Jul 12 '22 01:07 seceba

should be resolved in v0.1.6+1

rknell avatar Aug 19 '22 06:08 rknell