Comments (9)
Perhaps like this?
self
callback: [:value | | json |
json := [WAJsonParser parse: value] on: WAJsonSyntaxError do: [:ex | ex return: nil].
json isNil ifTrue: [self requestContext respond: [:resp | resp badRequest]].
aBlock value: json]
value: (JSJson new stringify: anObject)
from seaside.
Hmmm.. , 'null' is valid json and parses to nil. Perhaps this is better:
self
callback: [:value | | json |
json :=
[WAJsonParser parse: value]
on: WAJsonSyntaxError
do: [:ex | self requestContext respond: [:resp | resp badRequest]].
aBlock value: json]
value: (JSJson new stringify: anObject)
from seaside.
Hi @adriaon
The error should be caught by the Seaside exception handler, which will return an error 500.
There should not be a need for separate exception handlers inside (ajax) callbacks.
from seaside.
Since the urls for a callback:json:
callback are not intended to be part of a public api, the response to an invalid json can very well be an internal error (http status 500).
Though, if one wants to return a bad request (status 400), it should be possible to catch the WAJsonSyntaxError
in the configured exception handler and return such a response.
As such, I don't think we should alter the behavior of the callback. Similar situations can arise in all callbacks that accept values sent from the client. If they are tampered with and an error results, it should be considered normal.
Perhaps there are other reasons but as such, I don't think we should alter the callback implementation.
from seaside.
Hi @jbrichau ,
Thanks for your replies. Some considerations and thoughts from my end.
We see errors 500 as indications of 'bugs' or 'shortcomings' in our server. They are monitored and they are bad for our reputation.
JQAjax>>#callback:json: is the only place in Seaside where WAJsonParser is used (in my image), besides REST APIs.
JQAjax>>#callback:json: might be the only place where programmers can't do any sanity checks before the input gets processed, even-though it is 'just' parsing.
Dealing with WAJsonSyntaxError in the configured exception handler broadens the scope (enormously), which I think is undesirable. To compensate this, the configured error handler needs to be even more complex.
Those points made me think JQAjax>>#callback:json: is the best place to deal with it.
Cheers,
Adriaan.
from seaside.
Hey @adriaon,
Well, in my opinion, when a callback:json:
gets passed illegal json, then this is actually a bug. The trouble is that humans tampering with the code can trigger the error as well. So, I understand your point of view but it does mean you will need to log these errors in some other way when they occur.
I can imagine adding a new method with an optional error handler block (e.g. callback:json:onInvalidJson:
) to allow handling a json parse error differently, but I would not want to change the existing behaviour of the callback and start missing programming error in the server log.
from seaside.
Hi @jbrichau,
Fair enough. I make my own method. Probably named #callback:untrustedJson:. Because with #callback:json:onInvalidJson: all senders need to supply the same error handling block.
Cheers.
Adriaan.
from seaside.
Hey @adriaon
I'm not trying to reject adding this possibility to Seaside. I am, however, making the point that we should make this change optional and preserve existing behavior.
We can add callback:json:catchJsonParseError:
with the last argument being a Boolean
and make the original method delegate to this new method with false
as last argument.
from seaside.
Hi @jbrichau,
No worries. Didn't want to come across harsh. We are just pair-programming :-). Preserving existing behaviour is perfectly fine. Adding #callback:json:catchJsonParseError: is also a possibility. But the one with a block is more flexible. I thought #callback:json:onInvalidJson: is ok and then a shorthand method like #callback:untrustedJson: that calls #callback:json:onInvalidJson: with a default block. For me, either solution is fine.
from seaside.
Related Issues (20)
- Add option to WAResponse>>#document: to send documents without (re)encoding them HOT 7
- css in WAWelcomeFiles uses `.checkbox` class which was removed HOT 2
- JsJSONParser was deprecated in 3.1, it still exists HOT 1
- testCORSFilterFunctionalTest is broken on github CI HOT 3
- Dictionary>>#asJson does not produce valid JSON when using integer keys HOT 6
- WAGettextExporter catalog generation issues
- It is not fully clear why the session cache settings are described two times at /config
- WAPharoInspector cannot evaluate (compile) expressions in Pharo 11 HOT 1
- WAFastCGIAdaptor doesn’t write cookie attributes ‘Max-Age’ and ‘SameSite’ HOT 2
- WAFastCGIRequestConverter>>#requestUrlFor: signals a WAInvalidUrlSyntaxError when the ‘REQUEST_URI’ is ‘/?://’
- Nested script tags are not properly escaped HOT 2
- PackageManifest extensions should not be loaded in Squeak HOT 4
- Squeak does not have an implementation of Integer>>#asByteArray HOT 9
- Squeak WAVNCController>>#uiProcess should use Project, not UIManager
- Squeak has no #isImmediateObject HOT 4
- Squeak needs variant implementation of #openDebuggerOn: HOT 3
- Squeak WAVersionUploader>>#newVersion: needs to make sure the RWBinaryOrTextStream is ascii HOT 2
- Squeak WAWebServerAdaptor>>#responseFrom: sends #oldNetscapeString HOT 3
- WAAdmin clearSessions does not clear sessions on GemStone
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from seaside.