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

RequestPushMessageDeliveryAsync Fails with Unable to write content to request stream; content would exceed Content-Length. #30

Closed
twurm opened this issue Jan 17, 2024 · 5 comments · Fixed by #31
Assignees
Labels

Comments

@twurm
Copy link

twurm commented Jan 17, 2024

We've been using the lib for a long time. In our .Net 8 upgrade testing we started seeing the following:

System.Net.Http.HttpRequestException: Unable to write content to request stream; content would exceed Content-Length.
   at Lib.Net.Http.EncryptedContentEncoding.Aes128GcmEncoding.EncryptContentAsync(Stream source, Stream destination, Int32 recordSize, Byte[] contentEncryptionKeyInfoParameterHash, Byte[] nonceInfoParameterHash)
   at Lib.Net.Http.EncryptedContentEncoding.Aes128GcmEncoding.EncodeAsync(Stream source, Stream destination, Byte[] salt, Byte[] key, Byte[] keyId, Int32 recordSize)
   at Lib.Net.Http.EncryptedContentEncoding.Aes128GcmEncodedContent.SerializeToStreamAsync(Stream stream, TransportContext context)
   at System.Net.Http.HttpContent.g__WaitAsync|56_0(ValueTask copyTask)
   at System.Net.Http.HttpConnection.SendRequestContentAsync(HttpRequestMessage request, HttpContentWriteStream stream, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnection.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnection.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.SendWithVersionDetectionAndRetryAsync(HttpRequestMessage request, Boolean async, Boolean doRequestAuth, CancellationToken cancellationToken)
   at System.Net.Http.DiagnosticsHandler.SendAsyncCore(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at Microsoft.Extensions.Http.Logging.LoggingHttpMessageHandler.g__Core|5_0(HttpRequestMessage request, Boolean useAsync, CancellationToken cancellationToken)
   at Microsoft.Extensions.Http.Logging.LoggingScopeHttpMessageHandler.g__Core|5_0(HttpRequestMessage request, Boolean useAsync, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
   at Lib.Net.Http.WebPush.PushServiceClient.RequestPushMessageDeliveryAsync(PushSubscription subscription, PushMessage message, VapidAuthentication authentication, VapidAuthenticationScheme authenticationScheme, CancellationToken cancellationToken) in /_/src/Lib.Net.Http.WebPush/PushServiceClient.cs:line 189

Any thoughts on what Content is blowing past the Content-Length. We see this issue with both Microsoft and Google's push notification services.

@tpeczek
Copy link
Owner

tpeczek commented Jan 17, 2024

Hi @twurm,

Looks like it's coming from .NET HttpClient. Maybe some internal change/optimization to how content length is being handled which interferes with how the lib is doing things. I'll add .NET 8 to tests targets and see if I can catch it. If not, I'll probably ask for some minimal repro if possible.

@tpeczek tpeczek self-assigned this Jan 17, 2024
@tpeczek tpeczek added the Bug label Jan 17, 2024
@tpeczek tpeczek linked a pull request Jan 17, 2024 that will close this issue
@tpeczek
Copy link
Owner

tpeczek commented Jan 17, 2024

Hi @twurm,

I've added .NET 8 to tests targets and they all passed. I've also migrated the demo project to .NET 8 and tested it - it worked as expected.

This suggests to me that the issue you are experiencing is context-specific. I can try to investigate it, but I'll need as many details as possible - ideally a minimal reproduction.

@twurm
Copy link
Author

twurm commented Jan 17, 2024

@tpeczek I've been walking back changes all day it looks at the moment like when we switched from NewtonSoft to System.Text.Json as part of the upgrade we introduced the error. I haven't nailed down the serialization issue just yet but going back to NewtonSoft for serialization has solved the issue. Sorry to have tied up your time into this. Thanks for the help and the quick response.

When we nail down the serialization issue I'll update the ticket for anyone else that stumbles across it.

@twurm
Copy link
Author

twurm commented Jan 17, 2024

@tpeczek I've been walking back changes all day it looks at the moment like when we switched from NewtonSoft to System.Text.Json we introduced the error. I haven't nailed down the serializationn issue just yet but going back to NewtonSoft for serialization has solved the issue. Sorry to

@twurm twurm closed this as completed Jan 17, 2024
@tpeczek
Copy link
Owner

tpeczek commented Jan 18, 2024

Sorry to have tied up your time into this. Thanks for the help and the quick response.

No problem, in fact I should have add .NET 8 to tests targets anyway so it's good that you've motivated me.

When we nail down the serialization issue I'll update the ticket for anyone else that stumbles across it.

Please do. I'm honestly intrigued myself. Also it might be something that the library should sanitize or validate, so there might be some work to do here on my end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants