Pass WebAPI args as generic objects. Encode with correct HttpUtility.UrlEncode override

[azuisleet]

Breaking change. Rather than forcing everything into a string up front, keep them as generic objects. See #641

These comments need clarification:

// TODO: the WebAPI is a special snowflake that needs to appropriately handle url encoding
// this is in contrast to the steam3 content server APIs which use an entirely different scheme of encoding```

[JustArchi]

This looks OK at first, let's just not forget to document it properly in the changelog, something along:

Breaking change for WebAPI endpoints that accept rawbinary, you should now provide byte[] directly as an argument, as opposed to crafting encoded value yourself.
All other values will now be encoded in the request.

[JustArchi]

I'm just not sure if this change won't break stuff in regards to server-based interpretation.

Previously, I had this code:

await iSteamUserAuth.AuthenticateUser(
	encrypted_loginkey: Encoding.ASCII.GetString(WebUtility.UrlEncodeToBytes(encryptedLoginKey, 0, encryptedLoginKey.Length)),
	method: WebRequestMethods.Http.Post,
	sessionkey: Encoding.ASCII.GetString(WebUtility.UrlEncodeToBytes(encryptedSessionKey, 0, encryptedSessionKey.Length)),
	steamid: steamID
)

After this change, I expect to use this:

await iSteamUserAuth.AuthenticateUser(
	encrypted_loginkey: encryptedLoginKey,
	method: WebRequestMethods.Http.Post,
	sessionkey: encryptedSessionKey,
	steamid: steamID
)

I guess I'll just test this change once it's ready, feel free to ping me if you'd like me to.

[azuisleet]

Yes, the intention is to pull the encoding logic out of user code and into the library. You should be using the second example. I have not done extensive testing yet.

[codecov-io]

Codecov Report

Merging #668 into master will decrease coverage by 0.15%.
The diff coverage is 73.33%.

@@            Coverage Diff             @@
##           master     #668      +/-   ##
==========================================
- Coverage   23.68%   23.52%   -0.16%     
==========================================
  Files          85       85              
  Lines        8720     8726       +6     
  Branches      718      719       +1     
==========================================
- Hits         2065     2053      -12     
- Misses       6525     6545      +20     
+ Partials      130      128       -2

Impacted Files	Coverage Δ
SteamKit2/SteamKit2/Steam/WebAPI/SteamDirectory.cs	0% <0%> (ø)	⬆️
SteamKit2/SteamKit2/Steam/WebAPI/WebAPI.cs	43.45% <78.57%> (+1.63%)	⬆️
SteamKit2/SteamKit2/Util/Utils.cs	28.04% <0%> (-10.98%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7586b38...49f3382. Read the comment docs.

[JustArchi]

I have not done extensive testing yet.

Something is wrong since it's no longer possible to make ISteamUserAuth/AuthenticateUser work in any form. Steam responds with 400 bad request when supplied with byte[] arguments as in my example above. Could you take a look? I couldn't make it work through any means. I have login code in ASF if you'd like full login procedure. I followed the instructions in the release (see JustArchiNET/ArchiSteamFarm@ec95dc4).

[JustArchi]

In any case, just ensure the above is solved before triggering any stable release, since right now it's not just a regression but regression with no means of correction (@yaakov-h).

[xPaw]

For what it's worth, the non dynamic method works for me:

https://github.com/SteamDatabase/SteamDatabaseBackend/blob/1a26defd9750006f4dac553a7bec6deef7de160d/Steam/WebAuth.cs#L70-L77

[JustArchi]

You gave me good enough reason to ditch dynamic calls and just use non-dynamic ones instead, dynamic ones feel far more problematic than what it's worth 🤔. I'll see if it fixes my issue (and if yes, we'll know where the issue is). Thanks @xPaw!

[JustArchi]

I can confirm what @xPaw said above (thank you!), the issue doesn't exist when using CallAsync() (or Call() for synchronous), only when using dynamic interface. This should help pinpoint the issue. I've also decided to go non-dynamic way now.

[azuisleet]

Check fa128f1

The setup of the dictionary should be the same now. It was enumerating byte[] arrays.

[JustArchi]

Thanks for the quick fix! ❤️

[yaakov-h]

@JustArchi A new build is rolling out now, should be live on NuGet by the time you read this.