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

Update RateLimit parsing for Multiple values #236

Merged
merged 3 commits into from
Jul 10, 2023

Conversation

instanceof
Copy link
Contributor

@instanceof instanceof commented Jun 30, 2023

  • Multiple X-Rate-Limit headers can be combined with commas when using HttpURLResponse.value(forHTTPHeaderField:)
  • The comma and extra spaces show up in the RateLimit.entries keys.
x-rate-limit: user-hour-lim:3600;user-hour-rem:3598;
x-rate-limit: user-minute-lim:150;user-minute-rem:149;

becomes:

user-hour-lim:3600;user-hour-rem:3598;, user-minute-lim:150;user-minute-rem:149;
                                      ^^
print("Rate Limit: \(rateLimit.entries)")

Which was parsed as:

Rate Limit: ["user-hour-rem": 3598, ", user-minute-lim": 150, "user-hour-lim": 3600, "user-minute-rem": 149]
or
Rate Limit: [", user-minute-lim": 150, "user-hour-lim": 3600, "user-hour-rem": 3598, "user-minute-rem": 149]
  • Adds a test for the observed comma space separated headers observed.

See #227

@instanceof instanceof requested a review from AvdLee as a code owner June 30, 2023 18:02
@SwiftLeeBot
Copy link
Collaborator

SwiftLeeBot commented Jul 3, 2023

Warnings
⚠️ 'AppPrice' is deprecated: Deprecated
⚠️ 'AppPrice' is deprecated: Deprecated
⚠️ 'Prices' is deprecated: Deprecated
⚠️ 'AvailableTerritories' is deprecated: Deprecated
⚠️ 'InAppPurchases' is deprecated: Deprecated
⚠️ 'Prices' is deprecated: Deprecated
⚠️ 'AvailableTerritories' is deprecated: Deprecated
⚠️ 'InAppPurchases' is deprecated: Deprecated
⚠️ 'Builds' is deprecated: Deprecated
⚠️ 'Builds' is deprecated: Deprecated
⚠️ 'AgeRatingDeclaration' is deprecated: Deprecated
⚠️ 'AgeRatingDeclaration' is deprecated: Deprecated
⚠️ 'AppStoreVersionExperiment' is deprecated: Deprecated
⚠️ 'AppStoreVersionExperiment' is deprecated: Deprecated
⚠️ 'AppStoreVersionSubmission' is deprecated: Deprecated
⚠️ 'AppStoreVersionExperiment' is deprecated: Deprecated
⚠️ 'AppStoreVersionSubmission' is deprecated: Deprecated
⚠️ 'AppStoreVersionExperiment' is deprecated: Deprecated
⚠️ 'Prices' is deprecated: Deprecated
⚠️ 'AvailableTerritories' is deprecated: Deprecated
⚠️ 'Prices' is deprecated: Deprecated
⚠️ 'AvailableTerritories' is deprecated: Deprecated
⚠️ 'AppPrice' is deprecated: Deprecated
⚠️ 'AppStoreVersionExperiment' is deprecated: Deprecated
⚠️ 'AppStoreVersionExperiment' is deprecated: Deprecated
⚠️ 'Builds' is deprecated: Deprecated
⚠️ 'AgeRatingDeclaration' is deprecated: Deprecated
⚠️ 'AppStoreVersionExperiment' is deprecated: Deprecated
⚠️ 'AppStoreVersionExperiment' is deprecated: Deprecated
⚠️ 'AppPrice' is deprecated: Deprecated
⚠️ Left side of nil coalescing operator '??' has non-optional type 'String', so the right side is never used
⚠️ 'Prices' is deprecated: Deprecated
⚠️ 'AvailableTerritories' is deprecated: Deprecated
⚠️ 'InAppPurchases' is deprecated: Deprecated
⚠️ 'AppStoreVersionSubmission' is deprecated: Deprecated
⚠️ 'AppStoreVersionExperiment' is deprecated: Deprecated
⚠️ 'AppStoreVersionSubmission' is deprecated: Deprecated
⚠️ 'AppStoreVersionExperiment' is deprecated: Deprecated
⚠️ 'Prices' is deprecated: Deprecated
⚠️ 'AvailableTerritories' is deprecated: Deprecated
⚠️ 'AppStoreVersionExperiment' is deprecated: Deprecated
⚠️ 'AppStoreVersionExperiment' is deprecated: Deprecated
Messages
📖 AppStoreConnect-Swift-SDK-Tests: Executed 17 tests (0 failed, 0 retried, 0 skipped) in 0.107 seconds
📖

View more details on Bitrise

Code Coverage Report

Name Coverage

Generated by 🚫 Danger Swift against 8c730a6

@instanceof instanceof force-pushed the rate-limit-parsing branch from 51ef217 to a547788 Compare July 3, 2023 18:48
Copy link
Owner

@AvdLee AvdLee left a comment

Choose a reason for hiding this comment

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

Interesting, thanks for your contribution!

@instanceof
Copy link
Contributor Author

@instanceof
Copy link
Contributor Author

Any other changes requested to merge this PR?

@AvdLee AvdLee merged commit 2d7d7eb into AvdLee:master Jul 10, 2023
@AvdLee
Copy link
Owner

AvdLee commented Jul 10, 2023

Any other changes requested to merge this PR?

Nope, all good! Thanks again!

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.

3 participants