Repository navigation
Upgrade packages, bypass cache when no-store cache-control set, allow self-signed certs via appsettings.json for dev - #31
Closed
fluffynuts wants to merge 5 commits into
Closed
fluffynuts wants to merge 5 commits into
fluffynuts wants to merge 5 commits into
Conversation
…ured to do so - this would allow testing against a running local https asp.net application. Default is off, but that's up to the consumer to decide anyway
…ts, make https-invalid-cert test always runnable
- fix some issues - add tests around the fixes
fluffynuts
requested review from
devonCodeo and
jondrummond-codeo
and
a lite review from Copilot
September 21, 2026 08:58
fluffynuts
marked this pull request as ready for review
September 21, 2026 08:58
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A critical compilation issue and additional unresolved review findings remain.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
This PR upgrades dependencies, adds no-store request isolation, supports configurable invalid TLS certificates for development, and improves settings parsing and tests.
Changes:
- Adds
no-storeconcurrency handling. - Adds development SSL-certificate bypass configuration.
- Updates parsing, sanitization, packages, build configurations, and tests.
Review findings:
- Critical (1 vote):
SelfSignedImageServer.cslacks compatible ASP.NET Core references and will not compile. - Moderate (1 vote):
package.jsonuses anet7.0path while the server targetsnet8.0. - Moderate (1 vote): A
StreamAndHeaderstest result is not disposed. - Moderate (2 votes): The WebRequest SSL-bypass path is not covered by CI.
- Nit (1 vote): The SSL bypass setting is not documented in the README.
| File | Summary |
|---|---|
src/IRAAS/Startup.cs |
Gates environment dumping. |
src/IRAAS/Middleware/ConcurrencyMiddleware.cs |
Handles no-store requests. |
src/IRAAS/IRAAS.csproj |
Updates packages and configurations. |
src/IRAAS/ImageProcessing/UrlFetcher.cs |
Adds invalid SSL handling. |
src/IRAAS/ImageProcessing/DefaultImageResizeParameters.cs |
Improves parameter sanitization. |
src/IRAAS/IAppSettings.cs |
Adds SSL configuration. |
src/IRAAS/AppSettingsProvider.cs |
Updates parsing and defaults. |
src/IRAAS/appsettings.json |
Adds the SSL setting default. |
src/IRAAS/appsettings.deploy.json |
Adds the deployment placeholder. |
src/IRAAS.Tests/TestBase.cs |
Clears cached settings. |
src/IRAAS.Tests/Middleware/TestImageProviderErrorMiddleware.cs |
Updates assertions and formatting. |
src/IRAAS.Tests/Middleware/TestConcurrencyMiddleware.cs |
Tests no-store behavior. |
src/IRAAS.Tests/IRAAS.Tests.csproj |
Updates dependencies and configurations. |
src/IRAAS.Tests/ImageProcessing/TestUrlFetcher.cs |
Adds SSL tests and helpers. |
src/IRAAS.Tests/ImageProcessing/TestImageResizer.cs |
Updates URL assertions. |
src/IRAAS.Tests/ImageProcessing/TestImageResizeParameters.cs |
Updates URI assertions. |
src/IRAAS.Tests/ImageProcessing/TestDefaultImageResizeParameters.cs |
Adds sanitization tests. |
src/IRAAS.Tests/ImageProcessing/TestAppSettingsProvider.cs |
Adds configuration tests. |
src/IRAAS.Tests/ImageProcessing/SelfSignedImageServer.cs |
Adds an HTTPS test server. |
src/IRAAS.Tests/ImageProcessing/SelfSignedCertificate.cs |
Creates test certificates. |
src/IRAAS.StressTest/IRAAS.StressTest.csproj |
Updates configurations and dependencies. |
src/IRAAS.StressTest.ImageServer/IRAAS.StressTest.ImageServer.csproj |
Adds build configurations. |
src/IRAAS.sln |
Adds solution configurations. |
package.json |
Updates build, start, and test scripts. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+43
to
+52
| var builder = WebApplication.CreateSlimBuilder(); | ||
| builder.Logging.ClearProviders(); | ||
| var port = PortFinder.FindOpenPort(); | ||
| builder.WebHost.ConfigureKestrel( | ||
| options => | ||
| { | ||
| options.Listen( | ||
| IPAddress.Loopback, | ||
| port, | ||
| (ListenOptions listenOptions) => listenOptions.UseHttps(certificate) |
Comment on lines
+121
to
+125
| if (_appSettings.AllowInvalidSslCertificates) | ||
| { | ||
| if (req is HttpWebRequest httpRequest) | ||
| { | ||
| [HttpStatusCode.NotModified] = ThrowNotModifiedException, | ||
| }; | ||
| httpRequest.ServerCertificateValidationCallback = WebRequestSslHandler; |
devonCodeo
reviewed
Sep 21, 2026
devonCodeo
left a comment
devonCodeo
left a comment
There was a problem hiding this comment.
Just awaiting Copilot comments to be actioned, everything else looks good 👍
Collaborator
Author
|
superceded by #32 - closing out. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Does what it says on the tin:
cache-control: no-storeheader is retrieved, do not allow that request to bere-used by another with the same url. In this way, callers can pass through session identifier cookies and not
have them potentially bleed out to other concurrent requests