csgillespie · Oct 16, 2026 · Oct 16, 2026 · Oct 16, 2026
diff --git 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/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/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

diff --git 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
@@ -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
@@ -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
@@ -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
diff --git 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
@@ -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
@@ -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")