Allow to pass resource path that starts with .
#2981
Conversation
This will close #2980
|
|
|
Not sure why this is failing here: https://github.com/rstudio/shiny/pull/2981/checks?check_run_id=905762561 |
There was a problem hiding this comment.
I support this change, but have a couple comments. The test failure is unrelated: it's happening because the R-devel builds for Mac OS X have disappeared from https://mac.r-project.org/
R/server.R
Outdated
| @@ -71,7 +71,7 @@ registerClient <- function(client) { | |||
| #' @export | |||
| addResourcePath <- function(prefix, directoryPath) { | |||
| if (length(prefix) != 1) stop("prefix must be of length 1") | |||
| if (!grepl('^[a-z0-9\\-_][a-z0-9\\-_.]*$', prefix, ignore.case = TRUE, perl = TRUE)) { | |||
| if (!grepl('^[a-z0-9\\-_\\.][a-z0-9\\-_.]*$', prefix, ignore.case = TRUE, perl = TRUE)) { | |||
There was a problem hiding this comment.
I think this can be simplified to just:
'^[a-z0-9\\-_.]+$'
However, we should also disallow prefixes that consist only of . characters - that could lead to unnecessary confusion.
|
Ah, I hadn't think about the use of dot only. I've pushed a new test to check if the prefix is made only of dots (that would also cacth fn <- function(prefix) {
if (grepl("^\\.+$", prefix)) stop("prefix can't be composed of dots only") else "ok"
}
fn(".")
#> Error in fn("."): prefix can't be composed of dots only
fn("..")
#> Error in fn(".."): prefix can't be composed of dots only
fn(".well-known")
#> [1] "ok"Created on 2020-07-25 by the reprex package (v0.3.0) |
R/server.R
Outdated
| @@ -72,7 +72,7 @@ registerClient <- function(client) { | |||
| addResourcePath <- function(prefix, directoryPath) { | |||
| if (length(prefix) != 1) stop("prefix must be of length 1") | |||
| if (grepl("^\\.+$", prefix)) stop("prefix can't be composed of dots only") | |||
| if (!grepl('^[a-z0-9\\-_\\.][a-z0-9\\-_.]*$', prefix, ignore.case = TRUE, perl = TRUE)) { | |||
| if (!grepl('[a-z0-9\\-_.]*$', prefix, ignore.case = TRUE, perl = TRUE)) { | |||
There was a problem hiding this comment.
This needs a + instead of * -- otherwise it'll allow empty strings.
|
Argh, sure. Done in the last commit ✅ |
This will close #2980