From eccb6d0068fc64c9222d295f6ae552cf9018def7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Enr=C3=ADquez?= Date: Tue, 10 Jan 2023 16:32:51 +0100 Subject: [PATCH] [#25] Handle 304 redirect Problem: After recent work on Xrefcheck redirect behavior, it remained to discuss how to handle 304 redirects. Solution: With the current default configuration, 304 redirects are considered as valid, which seems appropriate taking into account that 304 responses usually mean that you previously received a successful response for that same request and there is no need to retransmit the resource again. We add a test case for this default config and update the FAQ section. --- README.md | 2 +- tests/Test/Xrefcheck/RedirectDefaultSpec.hs | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 3239f05..f9e67c6 100644 --- a/README.md +++ b/README.md @@ -143,7 +143,7 @@ There are several ways to fix this: 1. How does `xrefcheck` handle redirects? * The rules from the default configuration are as follows: * Permanent redirects (i.e. 301 and 308) are reported as errors. - * Temporary redirects (i.e. 302, 303 and 307) are assumed to be valid. + * There are no rules for other redirects, except for a special GitLab case, so they are assumed to be valid. * Redirect rules can be specified with the `externalRefRedirects` parameter within `networking`, which accepts an array of rules with keys `from`, `to`, `on` and `outcome`. The rule applied is the first one that matches with the `from`, `to` and `on` fields, if any, where diff --git a/tests/Test/Xrefcheck/RedirectDefaultSpec.hs b/tests/Test/Xrefcheck/RedirectDefaultSpec.hs index 2b9bd66..6a568cd 100644 --- a/tests/Test/Xrefcheck/RedirectDefaultSpec.hs +++ b/tests/Test/Xrefcheck/RedirectDefaultSpec.hs @@ -23,8 +23,9 @@ import Xrefcheck.Verify test_redirectRequests :: TestTree test_redirectRequests = testGroup "Redirect response defaults" - [ testGroup "Temporary" $ temporaryRedirectTests <$> [302, 303, 307] + [ testGroup "Temporary" $ allowedRedirectTests <$> [302, 303, 307] , testGroup "Permanent" $ permanentRedirectTests <$> [301, 308] + , testGroup "304 Not Modified" $ allowedRedirectTests <$> [304] ] where url :: Text @@ -33,11 +34,11 @@ test_redirectRequests = testGroup "Redirect response defaults" location :: Maybe Text location = Just "http://127.0.0.1:5000/other" - temporaryRedirectTests :: Int -> TestTree - temporaryRedirectTests statusCode = + allowedRedirectTests :: Int -> TestTree + allowedRedirectTests statusCode = redirectTests (show statusCode <> " passes by default") - (mkStatus statusCode "Temporary redirect") + (mkStatus statusCode "Allowed redirect") (\case Nothing -> Just $ RedirectMissingLocation $ fromList [url] Just _ -> Nothing