Skip to content

Commit

Permalink
Make resp_stream_lines respect encoding
Browse files Browse the repository at this point in the history
  • Loading branch information
jcheng5 committed Sep 6, 2024
1 parent 99171c8 commit 84252a9
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 7 deletions.
15 changes: 10 additions & 5 deletions R/req-perform-stream.R
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,11 @@ resp_stream_lines <- function(resp, lines = 1, max_size = Inf, warn = TRUE) {
return(character())

Check warning on line 240 in R/req-perform-stream.R

View check run for this annotation

Codecov / codecov/patch

R/req-perform-stream.R#L240

Added line #L240 was not covered by tests
}

encoding <- resp_encoding(resp)

lines_read <- character(0)

Check warning on line 245 in R/req-perform-stream.R

View check run for this annotation

Codecov / codecov/patch

R/req-perform-stream.R#L245

Added line #L245 was not covered by tests
while (lines > 0) {
line <- resp_stream_oneline(resp, max_size, warn)
line <- resp_stream_oneline(resp, max_size, warn, encoding)

Check warning on line 247 in R/req-perform-stream.R

View check run for this annotation

Codecov / codecov/patch

R/req-perform-stream.R#L247

Added line #L247 was not covered by tests
if (length(line) == 0) {
# No more data, either because EOF or req_perform_connection(blocking=FALSE).
# Either way, return what we have

Check warning on line 250 in R/req-perform-stream.R

View check run for this annotation

Codecov / codecov/patch

R/req-perform-stream.R#L249-L250

Added lines #L249 - L250 were not covered by tests
Expand All @@ -254,7 +256,7 @@ resp_stream_lines <- function(resp, lines = 1, max_size = Inf, warn = TRUE) {
lines_read
}

Check warning on line 258 in R/req-perform-stream.R

View check run for this annotation

Codecov / codecov/patch

R/req-perform-stream.R#L253-L258

Added lines #L253 - L258 were not covered by tests
resp_stream_oneline <- function(resp, max_size, warn) {
resp_stream_oneline <- function(resp, max_size, warn, encoding) {
repeat {
line_bytes <- resp_boundary_pushback(resp, max_size, find_line_boundary, include_trailer = TRUE)
if (is.null(line_bytes)) {
Expand All @@ -280,9 +282,13 @@ resp_stream_oneline <- function(resp, max_size, warn) {
`resp$body` <- line_bytes
line_con <- rawConnection(`resp$body`)
on.exit(close(line_con))
# TODO: Use iconv to convert from whatever encoding is specified in the

# readLines chomps the trailing newline. I assume this is desirable.

Check warning on line 286 in R/req-perform-stream.R

View check run for this annotation

Codecov / codecov/patch

R/req-perform-stream.R#L284-L286

Added lines #L284 - L286 were not covered by tests
raw_text <- readLines(line_con, n = 1, warn = warn)

# Use iconv to convert from whatever encoding is specified in the

Check warning on line 289 in R/req-perform-stream.R

View check run for this annotation

Codecov / codecov/patch

R/req-perform-stream.R#L289

Added line #L289 was not covered by tests
# response header, to UTF-8
return(readLines(line_con, n = 1, warn = warn))
return(iconv(raw_text, encoding, "UTF-8"))
}
}

Check warning on line 294 in R/req-perform-stream.R

View check run for this annotation

Codecov / codecov/patch

R/req-perform-stream.R#L293-L294

Added lines #L293 - L294 were not covered by tests
Expand Down Expand Up @@ -437,7 +443,6 @@ resp_boundary_pushback <- function(resp, max_size, boundary_func, include_traile
#' bytes has been exceeded without a line/event boundary, an error is thrown.
#' @export
#' @rdname resp_stream_raw
# TODO: max_size
resp_stream_sse <- function(resp, max_size = Inf) {
event_bytes <- resp_boundary_pushback(resp, max_size, find_event_boundary, include_trailer = FALSE)
if (!is.null(event_bytes)) {
Expand Down
7 changes: 5 additions & 2 deletions tests/testthat/test-req-perform-stream.R
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ test_that("handles line endings of multiple kinds", {
app <- webfakes::new_app()

app$get("/events", function(req, res) {
res$set_header("Content-Type", "text/plain; charset=Shift_JIS")
res$send_chunk(as.raw(c(0x82, 0xA0, 0x0A)))
Sys.sleep(0.1)
res$send_chunk("crlf\r\n")
Sys.sleep(0.1)
res$send_chunk("lf\n")
Expand All @@ -99,7 +102,7 @@ test_that("handles line endings of multiple kinds", {
resp1 <- req_perform_connection(req, blocking = TRUE)
withr::defer(close(resp1))

for (expected in c("crlf", "lf", "cr", "half line/other half", "broken crlf", "another line")) {
for (expected in c("\u3042", "crlf", "lf", "cr", "half line/other half", "broken crlf", "another line")) {
rlang::inject(expect_equal(resp_stream_lines(resp1), !!expected))
}
expect_warning(
Expand All @@ -112,7 +115,7 @@ test_that("handles line endings of multiple kinds", {
resp2 <- req_perform_connection(req, blocking = FALSE)
withr::defer(close(resp2))

for (expected in c("crlf", "lf", "cr", "half line/other half", "broken crlf", "another line")) {
for (expected in c("\u3042", "crlf", "lf", "cr", "half line/other half", "broken crlf", "another line")) {
repeat {
out <- resp_stream_lines(resp2)
if (length(out) > 0) {
Expand Down

0 comments on commit 84252a9

Please sign in to comment.