chucknorris-io / chuck-api

👊 chucknorris.io is a free resource for hand curated Chuck Norris facts.
https://api.chucknorris.io
GNU General Public License v3.0
239 stars 29 forks source link

Return a HTTP 400 if query parameters are empty string #24

Open matchilling opened 5 years ago

matchilling commented 5 years ago
$ curl -v https://api.chucknorris.io/jokes/random?category=
*   Trying 104.28.12.58...
* TCP_NODELAY set
* Connected to api.chucknorris.io (104.28.12.58) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* Cipher selection: ALL:!EXPORT:!EXPORT40:!EXPORT56:!aNULL:!LOW:!RC4:@STRENGTH
* successfully set certificate verify locations:
*   CAfile: /etc/ssl/cert.pem
  CApath: none
* TLSv1.2 (OUT), TLS handshake, Client hello (1):
* TLSv1.2 (IN), TLS handshake, Server hello (2):
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
* TLSv1.2 (IN), TLS handshake, Server finished (14):
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.2 (OUT), TLS change cipher, Client hello (1):
* TLSv1.2 (OUT), TLS handshake, Finished (20):
* TLSv1.2 (IN), TLS change cipher, Client hello (1):
* TLSv1.2 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.2 / ECDHE-ECDSA-CHACHA20-POLY1305
* ALPN, server accepted to use h2
* Server certificate:
*  subject: OU=Domain Control Validated; OU=PositiveSSL Multi-Domain; CN=sni233687.cloudflaressl.com
*  start date: Jun 13 00:00:00 2019 GMT
*  expire date: Dec 20 23:59:59 2019 GMT
*  subjectAltName: host "api.chucknorris.io" matched cert's "*.chucknorris.io"
*  issuer: C=GB; ST=Greater Manchester; L=Salford; O=COMODO CA Limited; CN=COMODO ECC Domain Validation Secure Server CA 2
*  SSL certificate verify ok.
* Using HTTP2, server supports multi-use
* Connection state changed (HTTP/2 confirmed)
* Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0
* Using Stream ID: 1 (easy handle 0x7fb4d880c600)
> GET /jokes/random?category= HTTP/2
> Host: api.chucknorris.io
> User-Agent: curl/7.54.0
> Accept: */*
> 
* Connection state changed (MAX_CONCURRENT_STREAMS updated)!
< HTTP/2 404 
< date: Thu, 13 Jun 2019 23:09:02 GMT
< content-type: application/json;charset=UTF-8
< set-cookie: __cfduid=d409ce228b0dd5a252d89fe30b3dd0cff1560467341; expires=Fri, 12-Jun-20 23:09:01 GMT; path=/; domain=.chucknorris.io; HttpOnly
< via: 1.1 vegur
< expect-ct: max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct"
< server: cloudflare
< cf-ray: 4e67bad6bddd2d33-TXL
< 
* Connection #0 to host api.chucknorris.io left intact
{"timestamp":"2019-06-13T23:09:02.042Z","status":404,"error":"Not Found","message":"No jokes for category \"\" found.","path":"/jokes/random"}
a2937 commented 1 year ago

It needs more checks for empty strings. Ex)

https://github.com/chucknorris-io/chuck-api/blob/478d09f0db47b24eeae92725ef978aaecbcadaf3/src/main/java/io/chucknorris/api/joke/JokeController.java#L140

should be if ((categoryString == null || categoryString.trim().isEmpty()) && (name == null || name.trim().isEmpty()))

https://github.com/chucknorris-io/chuck-api/blob/478d09f0db47b24eeae92725ef978aaecbcadaf3/src/main/java/io/chucknorris/api/joke/JokeController.java#L144

should be if ((categoryString == null || categoryString.trim().isEmpty()) && (name != null && !name.trim().isEmpty()))

though it can probably be simplified to (categoryString == null || categoryString.trim().isEmpty())

https://github.com/chucknorris-io/chuck-api/blob/478d09f0db47b24eeae92725ef978aaecbcadaf3/src/main/java/io/chucknorris/api/joke/JokeController.java#L162

should ideally be if ((name != null && !name.trim().isEmpty())).

I'll happily provide my changes in a PR if you want and the relevant tests of course.