rexyai / RestRserve

R web API framework for building high-performance microservices and app backends
https://restrserve.org
271 stars 31 forks source link

ETag extensions and small bug fix #191

Closed DavZim closed 2 years ago

DavZim commented 2 years ago

Adds additional headers to ETag Middleware functionality and fixes a small bug in mime-types:

Furthermore, If-None-Match and If-Match allow "*" to match all.

Also encodes non 200 status codes as text/plain which fixes some issues for pdf types.

This PR also comes with tests for the new headers and corrected tests for the text/plain mime-type.

codecov[bot] commented 2 years ago

Codecov Report

Merging #191 (cbe221f) into dev (0da2e5b) will increase coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev     #191      +/-   ##
==========================================
+ Coverage   94.92%   94.93%   +0.01%     
==========================================
  Files          28       28              
  Lines        1378     1382       +4     
==========================================
+ Hits         1308     1312       +4     
  Misses         70       70              
Impacted Files Coverage Δ
R/ETagMiddleware.R 92.20% <100.00%> (+0.42%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0da2e5b...cbe221f. Read the comment docs.

dselivanov commented 2 years ago

Hm, why text/plain is important when body is null?

DavZim commented 2 years ago

There is an error when a file of mime-type "application/pdf" is "returned" with matching if-none-match header "can't encode body with content_type = 'application/pdf'. I suspect that the EncodeDecode Middleware cannot deal with NULL but mime-type "application/pdf".

To replicate:

library(RestRserve)

tmpdir = tempdir()
ff = file.path(tmpdir, "a-file.pdf")
pdf(ff)
plot(1:10, 1:10)
dev.off()

act_hash = digest::digest(file = ff, algo = "crc32")

app = Application$new()
etag = ETagMiddleware$new()
app$append_middleware(etag)

app$add_static("/static", tmpdir)

# backend = BackendRserve$new(headers_to_split = c("test-header", "accept"))
# backend$start(app, http_port = 8080)

rq = Request$new("/static/a-file.pdf", method = "GET", headers = list("If-None-Match" = act_hash))

rs = app$process_request(rq)

# with text/plain => GOOD, expected the 304
rs$content_type # application/pdf
rs$body         # ""
rs$status_code  # 304

# without text/plain => BAD, expected a 304 not a 500
rs$content_type # text/plain
rs$body         # 500 Internal Server Error: can't encode body with content_type = 'application/pdf'
rs$status_code  # 500