freme-project / basic-services

Apache License 2.0
0 stars 1 forks source link

[urlsupport filter] missing content-type causes error #112

Closed ArneBinder closed 7 years ago

ArneBinder commented 7 years ago

Try this request (but use Postman, curl sets a default content-type):

POST http://api-dev.freme-project.eu/current/toolbox/nif-converter?intype=url&input=http://beautifulberlin.blogspot.de/2009/07/short-history-of-berlin.html

I get:

{"timestamp":1475585730677,"status":500,"error":"Internal Server Error","exception":"java.lang.NullPointerException","message":"No message available","path":"/toolbox/nif-converter"}

This is because the page at http://beautifulberlin.blogspot.de/2009/07/short-history-of-berlin.html does not send a content-type header back, so the content-type variable is set to null here, but checked later with isEmpty(). In general, I prefere to use Strings.isNullOrEmpty(...) to avoid such a problem. @jnehring Or is this bad in any way?

Furthermore, it looks like there are cases that ModifyRequestBodyWrapper is initialised with contentType="", but ModifyRequestBodyWrapper defaults to text/html only, if it is null, not an empty string, see here.

jnehring commented 7 years ago

In general, I prefere to use Strings.isNullOrEmpty(...) to avoid such a problem. @jnehring Or is this bad in any way?

Strings.isNullOrEmpty is fine. In general you should treat null values.

I think the current behaviour is fine, except for it should not crash. When no content-type is specified then the system should treat it as turtle. The API call works when I specify informat=text/html so we need to deal with the nullpointerexception only.

Does the API documentation mention that intype=url guesses the input format from the content type header of the web page? I did not know about this behaviour. We should note it somewhere.

jnehring commented 7 years ago

This behaviour is documented in the full api documentation. Please add an example curl request and a short explanation to getting started for api users and link from the note in the api documentation to the article. Please add a link from the full api documentation in the line "url: The input is retrieved from the url specified in the 'input' or 'i' parameter" like "For more information see the Getting started for API users guide".

bgrusdt commented 7 years ago

I added the description , linked to it from the full api documentation and changed the code such that no NullPointerException is thrown anymore.

jnehring commented 7 years ago

thank you!

bgrusdt commented 7 years ago

@jnehring We decided to change the behaviour such that now text/turtle is used as default Content-Type if the user does not specify it. I have changed this. Now the call,

POST http://api-dev.freme-project.eu/current/toolbox/nif-converter?intype=url&input=http://beautifulberlin.blogspot.de/2009/07/short-history-of-berlin.html

results in a 'Error parsing NIF-Input' - Error which is appropriate.

I have also adapted the documentation.

jnehring commented 7 years ago

Thank you! Btw. it is not necessary to add a dynamic URL to each CURL example that uses freme-dev in the dev documentation and freme-live in the live documentation. You can point directly to the live version. But you can leave it like it is, it causes no problem. I just want to save you the extra work.

bgrusdt commented 7 years ago

Thank you @jnehring, thats good to know.