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

comm.httpPostRaw #4189

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

Conversation

reversefold
Copy link
Contributor

@reversefold reversefold commented Jan 15, 2025

Adds comm.httpPostRaw which supports setting the Content-Type and sends the supplied payload string verbatim instead of forcing it into a form field like httpPost.

Check if completed:

Note: Also includes #4188

@YoshiRulz

This comment was marked as resolved.

@reversefold

This comment was marked as resolved.

Copy link
Member

@YoshiRulz YoshiRulz left a comment

Choose a reason for hiding this comment

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

I'm not sure if the name "raw" is apt, since it's taking text rather than a bytestream.

{
return Post(
url ?? PostUrl,
new StringContent(payload, Encoding.UTF8, contentType),
Copy link
Member

Choose a reason for hiding this comment

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

I set up ContentObjectFor above for you—it should be simply

-#pragma warning disable BHI1005 // exception type
-				_ => throw new NotImplementedException()
+				/*MIME_PLAINTEXT,*/ _ => new StringContent(payload, Encoding.UTF8, mimeType)
-#pragma warning restore BHI1005

but I'm now realising it shouldn't have [ConstantExpected] and also that the method can be public static. I'll fix that in a minute.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I chose "raw" because it was all I could think of to differentiate from the existing one that submits a form. Technically that one takes text as well. This one just posts the text as the payload directly instead of forcing it into a form value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I hadn't seen the extra method. Not sure why it's really needed since there would still be different methods to pass in the different mimeTypes anyway....and for the raw post method I'm exposing the contentType header not to allow encoding the string differently but to allow the script author to set the header as appropriate. This library doesn't need to create a different content object if it's "text/plain" or "application/json". The incoming string is still just a string that's send as the body of the request as StringContent.

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