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

Code scanning fix patches #638

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Code scanning fix patches #638

wants to merge 2 commits into from

Conversation

64J0
Copy link
Member

@64J0 64J0 commented Jan 16, 2025

Description

Fix the following code scan warnings:

How to test

TODO :: add automated tests for ResponseCaching

@64J0 64J0 self-assigned this Jan 16, 2025
@@ -44,7 +44,7 @@ let mustAcceptAny (mimeTypes: string list) (optionalErrorHandler: OptionalErrorH
)

match Option.ofObj (headers.Accept :> _ seq) with
| Some xs when Seq.map (_.ToString()) xs |> Seq.exists (fun x -> Seq.contains x mimeTypes) -> next ctx
| Some xs when Seq.map (_.ToString()) xs |> Seq.exists (fun x -> List.contains x mimeTypes) -> next ctx
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick, you can probably move the ToString inside the Seq.exists.
And I wonder if Sets should be used here. The order of mimeTypes doesn't matter here.

@@ -55,14 +55,17 @@ module ResponseCaching =
| Public duration -> tHeaders.CacheControl <- cacheHeader true duration
| Private duration -> tHeaders.CacheControl <- cacheHeader false duration

if vary.IsSome then
headers.[HeaderNames.Vary] <- StringValues [| vary.Value |]
match vary with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Option.iter maybe?

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.

2 participants