Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opt in to trailing slash 307 redirect. Opt out of 405 logic. #746

Merged
merged 14 commits into from
Jan 5, 2021

Conversation

schloerke
Copy link
Collaborator

Fixes #100
Fixes #465

If a route is submitted without a trailing slash, and a comparable route exists with a trailing slash, then a 307 redirect is returned

options_plumber("redirect" = TRUE)

pr() %>% 
  pr_get("/get/", function(a) a) %>%
  pr_post("/post/", function(a) a) %>%
  pr_mount(
    "/mnt", 
    pr() %>% pr_get("/", function(a) a)
  ) %>% 
  pr_run(port = 8000)
$ curl "localhost:8000/mnt"
{"message":"307 - Redirecting with trailing slash"}

# follow redirect
$ curl "localhost:8000/mnt?a=42" -L
["42"]
curl "localhost:8000/mnt"
#> {"message":"307 - Redirecting with trailing slash"}

# follow redirect
curl "localhost:8000/mnt?a=42" -L
#> ["42"]

curl "localhost:8000/get?a=42"
#> {"message":"307 - Redirecting with trailing slash"}
curl "localhost:8000/get?a=42" -L
#> ["42"]

curl --data 'a=42' "localhost:8000/post"
#> {"message":"307 - Redirecting with trailing slash"}
#> ["42"]

PR task list:

  • Update NEWS
  • Add tests
  • Update documentation with devtools::document()

  • I am not sold on the new option names. Any ideas?
  • Is it ok to bring in ellipsis?
  • Is stopping ok if a bad option name is given or an unnamed option is given?
  • Is ok to put ... at the beginning of options_plumber()?

@schloerke schloerke added this to the v1.1.0 milestone Dec 30, 2020
@schloerke schloerke requested a review from cpsievert December 30, 2020 23:22
@schloerke schloerke changed the title Opt in to tailing slash redirects. Move 405 logic to always be used. Opt in to trailing slash redirects. Move 405 logic to always be used. Dec 31, 2020
R/options_plumber.R Outdated Show resolved Hide resolved
DESCRIPTION Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
R/options_plumber.R Outdated Show resolved Hide resolved
R/options_plumber.R Outdated Show resolved Hide resolved
tests/testthat/test-plumber.R Outdated Show resolved Hide resolved
tests/testthat/test-plumber.R Outdated Show resolved Hide resolved
R/options_plumber.R Outdated Show resolved Hide resolved
schloerke and others added 4 commits January 5, 2021 11:10
* master:
  Quote the version number in Dockerfile (#751)
  Do not use `plan(multiprocess)`. Instead, use `plan(multisession)` (#747)
  Default res headers to named list instead of list (#745)
  Add stage debugging section in Tips&Tricks (#741)
  Switch `Plumber$print()` wording to use magrittr api (#742)
@schloerke schloerke changed the title Opt in to trailing slash redirects. Move 405 logic to always be used. Opt in to trailing slash 307 redirect. Opt out of 405 logic. Jan 5, 2021
@schloerke schloerke merged commit e9dac93 into master Jan 5, 2021
@schloerke schloerke deleted the mnt_updates branch January 5, 2021 16:25
schloerke added a commit that referenced this pull request Jan 5, 2021
* master:
  Opt in to trailing slash 307 redirect. Opt out of 405 logic. (#746)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add mounted router / route to mounted location if no definition exists Redirect trailing slashes
2 participants