From 05cd4fca6114c19facc3e9cb71ac8eaf24501f02 Mon Sep 17 00:00:00 2001 From: Colin Gillespie Date: Fri, 16 Oct 2026 11:31:23 +0100 Subject: [PATCH 1/2] fix: Pass SSL redirection only when http ends on a working https page Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QVzHxqR2weizVuCKLw3fZm --- NAMESPACE | 1 - R/check_server_headers.R | 7 ------- R/check_ssl_functionality.R | 15 +++++++++++++-- R/documentation_links.R | 2 +- R/server_header_summaries.R | 15 --------------- man/header_summary.Rd | 3 --- tests/testthat/test-check_server_headers.R | 9 --------- tests/testthat/test-check_ssl_functionality.R | 16 ++++++++++++++++ tests/testthat/test-header_summary.R | 5 ----- 9 files changed, 30 insertions(+), 43 deletions(-) diff --git a/NAMESPACE b/NAMESPACE index 976c495..4487bdd 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -19,7 +19,6 @@ S3method(header_summary,"x-xss-protection") S3method(header_summary,cookies) S3method(header_summary,default) S3method(header_summary,nel) -S3method(header_summary,redirection) S3method(header_summary,scheme) S3method(header_summary,server) export(check) diff --git a/R/check_server_headers.R b/R/check_server_headers.R index dbf0536..942e0a0 100644 --- a/R/check_server_headers.R +++ b/R/check_server_headers.R @@ -18,7 +18,6 @@ get_response_headers = function(server) { headers, list( scheme = httr::parse_url(res$url)[["scheme"]], - redirection = is_redirected(res), `subresource-integrity` = get_js_resources(res), cookies = httr::cookies(res)$secure ) @@ -52,12 +51,6 @@ get_status_codes = function(res) { purrr::map_dbl(all_headers, "status") } -# Does the website redirection implemented i.e. HTTP -> HTTPS? -is_redirected = function(res) { - status_codes = get_status_codes(res) - http_versions = purrr::map_chr(res$all_headers, "version") - any(grepl("^3", status_codes)) || any(http_versions == "HTTP/2") -} # Retrieves HTML attributes of cross-origin script tags to check # subresource integrity diff --git a/R/check_ssl_functionality.R b/R/check_ssl_functionality.R index bd7525f..0689f06 100644 --- a/R/check_ssl_functionality.R +++ b/R/check_ssl_functionality.R @@ -13,11 +13,10 @@ check_ssl_redirect = function(clean_server) { status_codes = get_status_codes(res) status_code_msg = glue::glue(paste(status_codes, collapse = " {cli::symbol$arrow_right} ")) } else { - status_codes = NA status_code_msg = "Connection failed" } - if (length(status_codes) > 1 && any(stringr::str_starts(status_codes, "2"))) { + if (is_https_redirect(res)) { status = "OK" message = "SSL redirection successful" cli::cli_alert_success("{cli::col_green({message})}: http -> https") @@ -32,9 +31,21 @@ check_ssl_redirect = function(clean_server) { message = message, value = status_code_msg, primary_header = TRUE, + documentation = .documentation_links[["ssl-redirection"]] ) } +# An http request should end on an https page that loads (2xx) +is_https_redirect = function(res) { + if (is_ssl_error(res)) { + return(FALSE) + } + status_codes = get_status_codes(res) + final_status = status_codes[length(status_codes)] + final_scheme = tolower(httr::parse_url(res$url)$scheme) + identical(final_scheme, "https") && isTRUE(final_status >= 200 && final_status < 300) +} + is_ssl_error = function(res) inherits(res, "try-error") get_ssl_error = function(res) { rtn = list() diff --git a/R/documentation_links.R b/R/documentation_links.R index ecee446..72c5379 100644 --- a/R/documentation_links.R +++ b/R/documentation_links.R @@ -15,7 +15,7 @@ "content-security-policy" = "https://infosec.mozilla.org/guidelines/web_security#content-security-policy", "cookies" = "https://infosec.mozilla.org/guidelines/web_security#cookies", "permissions-policy" = "https://scotthelme.co.uk/goodbye-feature-policy-and-hello-permissions-policy/", - "redirection" = "https://infosec.mozilla.org/guidelines/web_security#http-redirections", + "ssl-redirection" = "https://infosec.mozilla.org/guidelines/web_security#http-redirections", "referrer-policy" = "https://infosec.mozilla.org/guidelines/web_security#referrer-policy", "strict-transport-security" = "https://infosec.mozilla.org/guidelines/web_security#http-strict-transport-security", "subresource-integrity" = "https://infosec.mozilla.org/guidelines/web_security#subresource-integrity", diff --git a/R/server_header_summaries.R b/R/server_header_summaries.R index 33fc07c..aa76e31 100644 --- a/R/server_header_summaries.R +++ b/R/server_header_summaries.R @@ -250,21 +250,6 @@ header_summary.scheme = function(value, ...) { dplyr::tibble(header = header, status = status, message = message, value = NA_character_) } -#' @rdname header_summary -#' @export -`header_summary.redirection` = function(value, ...) { - header = class(value) - value = as.logical(value) - if (value) { - status = "OK" - message = "Redirects to HTTPS" - } else { - status = "WARN" - message = "Does not redirect to an HTTPS site" - } - dplyr::tibble(header = header, status = status, message = message, value = NA_character_) -} - #' @rdname header_summary #' @export `header_summary.subresource-integrity` = function(value, ...) { diff --git a/man/header_summary.Rd b/man/header_summary.Rd index 7149c87..c466bfd 100644 --- a/man/header_summary.Rd +++ b/man/header_summary.Rd @@ -21,7 +21,6 @@ \alias{header_summary.x-permitted-cross-domain-policies} \alias{header_summary.x-powered-by} \alias{header_summary.cookies} -\alias{header_summary.redirection} \alias{header_summary.subresource-integrity} \title{Header Summaries} \usage{ @@ -63,8 +62,6 @@ header_summary(value, ...) \method{header_summary}{cookies}(value, ...) -\method{header_summary}{redirection}(value, ...) - \method{header_summary}{`subresource-integrity`}(value, ...) } \arguments{ diff --git a/tests/testthat/test-check_server_headers.R b/tests/testthat/test-check_server_headers.R index 7d8a855..31ff02a 100644 --- a/tests/testthat/test-check_server_headers.R +++ b/tests/testthat/test-check_server_headers.R @@ -1,12 +1,3 @@ -test_that("Is redirected works", { - response = function(status, version = "HTTP/1.1") { - list(all_headers = purrr::map2(status, version, \(s, v) list(status = s, version = v))) - } - expect_false(is_redirected(response(200))) - expect_true(is_redirected(response(c(301, 200)))) - expect_true(is_redirected(response(200, "HTTP/2"))) -}) - test_that("check() copes with a site that has no https", { skip_if_offline() skip_on_ci() diff --git a/tests/testthat/test-check_ssl_functionality.R b/tests/testthat/test-check_ssl_functionality.R index 874bdd3..0e5860f 100644 --- a/tests/testthat/test-check_ssl_functionality.R +++ b/tests/testthat/test-check_ssl_functionality.R @@ -36,3 +36,19 @@ test_that("Only a leading scheme is stripped, in any case", { "example.com/?next=https://other.com" ) }) + +test_that("An http request must end on a working https page", { + response = function(url, status) { + list(url = url, all_headers = lapply(status, \(s) list(status = s))) + } + expect_true(is_https_redirect(response("https://example.com/", c(301, 200)))) + expect_true(is_https_redirect(response("HTTPS://example.com/", c(301, 301, 204)))) + # Redirects to another http page + expect_false(is_https_redirect(response("http://www.example.com/", c(301, 200)))) + # No redirect at all + expect_false(is_https_redirect(response("http://example.com/", 200))) + # Ends on an error page + expect_false(is_https_redirect(response("https://example.com/", c(301, 404)))) + # Connection failed + expect_false(is_https_redirect(try(stop("Timeout"), silent = TRUE))) +}) diff --git a/tests/testthat/test-header_summary.R b/tests/testthat/test-header_summary.R index 3bf1102..a2b24d1 100644 --- a/tests/testthat/test-header_summary.R +++ b/tests/testthat/test-header_summary.R @@ -92,11 +92,6 @@ test_that("Cookies must all use the Secure flag", { expect_equal(summarise_header(c(TRUE, FALSE), "cookies")$status, "WARN") }) -test_that("Redirection is reported", { - expect_equal(summarise_header(TRUE, "redirection")$status, "OK") - expect_equal(summarise_header(FALSE, "redirection")$status, "WARN") -}) - test_that("Every cross-origin script needs integrity and crossorigin", { sri = function(value) summarise_header(value, "subresource-integrity") expect_match(sri(list())$message, "no cross-origin scripts") From 001baf572b4485e8a03be35a0ff28e092e98d99a Mon Sep 17 00:00:00 2001 From: Colin Gillespie Date: Fri, 16 Oct 2026 11:31:24 +0100 Subject: [PATCH 2/2] chore: Bump version to 0.1.18 --- DESCRIPTION | 2 +- NEWS.md | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 1509b07..d556a7f 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Type: Package Package: serverHeaders Title: Server Header Summary -Version: 0.1.17 +Version: 0.1.18 Authors@R: person("Jumping", "Rivers", , "info@jumpingrivers.com", role = c("aut", "cre")) Description: Checks and summarises the server headers for a given URL. diff --git a/NEWS.md b/NEWS.md index 11730ce..7e32739 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,10 @@ +# serverHeaders 0.1.18 + +## Bug fixes and minor improvements + +* `ssl-redirection` now passes only when the http request ends on an https page with a 2xx status. Before, any redirect chain containing a 2xx passed. +* The `redirection` row and `header_summary.redirection()` are gone. They treated any 3xx or HTTP/2 response as an HTTPS redirect, and duplicated `ssl-redirection`. + # serverHeaders 0.1.17 ## Bug fixes and minor improvements