Skip to content

Integration Tests: added tests to verify 304 behavior#2659

Merged
NachoSoto merged 2 commits into
mainfrom
integration-test-304
Jun 16, 2023
Merged

Integration Tests: added tests to verify 304 behavior#2659
NachoSoto merged 2 commits into
mainfrom
integration-test-304

Conversation

@NachoSoto

Copy link
Copy Markdown
Contributor

We had no end-to-end tests for ETags.

@NachoSoto NachoSoto added the test label Jun 15, 2023
@NachoSoto NachoSoto requested a review from a team June 15, 2023 16:58

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This has exposed a bug in signature validation on some 304 responses. I'm still debugging why.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Figured it out: #2668.

NachoSoto added a commit that referenced this pull request Jun 16, 2023
Changed this log:
```
[network] DEBUG: ℹ️ API request failed: GET /v1/subscribers/17571811-B0A4-49DC-B326-572E602BF195: Request failed signature verification.
```

To this:
```
[network] DEBUG: ℹ️ API request failed: GET /v1/subscribers/BD9F38FF-33C6-4A77-86F7-6AEA92E6717D (200): Request failed signature verification.
```

This was useful when debugging #2668 and #2659.
@NachoSoto NachoSoto force-pushed the integration-test-304 branch from 5c310e6 to a603c48 Compare June 16, 2023 01:49
@NachoSoto NachoSoto changed the base branch from main to http-client-disable-cache June 16, 2023 01:49
@codecov

codecov Bot commented Jun 16, 2023

Copy link
Copy Markdown

Codecov Report

Merging #2659 (9cf8b88) into main (6d1cf97) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2659      +/-   ##
==========================================
- Coverage   86.47%   86.47%   -0.01%     
==========================================
  Files         208      208              
  Lines       14783    14784       +1     
==========================================
  Hits        12784    12784              
- Misses       1999     2000       +1     
Impacted Files Coverage Δ
Sources/Networking/HTTPClient/HTTPClient.swift 98.67% <100.00%> (+0.27%) ⬆️

... and 2 files with indirect coverage changes

Turns out this was breaking a 304 integration test in #2659. I'm very confused why we hadn't seen it before.
Possibly it didn't matter for normal requests, but it was breaking signature verification because the `nonce` was different on the new request, and therefore it wouldn't match the cached signature.
@NachoSoto NachoSoto force-pushed the http-client-disable-cache branch from 284bd01 to 022e958 Compare June 16, 2023 17:10
@NachoSoto NachoSoto force-pushed the integration-test-304 branch from a603c48 to 9cf8b88 Compare June 16, 2023 17:10
Base automatically changed from http-client-disable-cache to main June 16, 2023 17:26
NachoSoto added a commit that referenced this pull request Jun 16, 2023
Turns out this was breaking an HTTP 304 integration test in #2659. I'm
very confused why we hadn't seen it before.

Possibly it didn't matter for normal requests, but it was breaking
signature verification because the `nonce` was different on the new
request, and therefore it wouldn't match the cached signature.
@NachoSoto NachoSoto merged commit fdcb64d into main Jun 16, 2023
@NachoSoto NachoSoto deleted the integration-test-304 branch June 16, 2023 17:28
NachoSoto pushed a commit that referenced this pull request Jun 22, 2023
**This is an automatic release.**

### Bugfixes
* `PurchasesOrchestrator`: update `CustomerInfoManager` cache after
processing transactions (#2676) via NachoSoto (@NachoSoto)
* `ErrorResponse`: drastically improved error messages, no more "unknown
error"s (#2660) via NachoSoto (@NachoSoto)
* `PaywallExtensions`: post purchases with `Offering` identifier (#2645)
via NachoSoto (@NachoSoto)
* Support `product_plan_identifier` for purchased subscriptions from
`Google Play` (#2654) via Josh Holtz (@joshdholtz)
### Performance Improvements
* `copy(with: VerificationResult)`: optimization to avoid copies (#2639)
via NachoSoto (@NachoSoto)
### Other Changes
* `ETagManager`: refactored e-tag creation and tests (#2671) via
NachoSoto (@NachoSoto)
* `getPromotionalOffer`: return `ErrorCode.ineligibleError` if receipt
is not found (#2678) via NachoSoto (@NachoSoto)
* `TimingUtil`: removed slow purchase logs (#2677) via NachoSoto
(@NachoSoto)
* `CI`: changed `Codecov` to `informational` (#2670) via NachoSoto
(@NachoSoto)
* `LoadShedderIntegrationTests`: verify requests are actually handled by
load shedder (#2663) via NachoSoto (@NachoSoto)
* `ETagManager.httpResultFromCacheOrBackend`: return response headers
(#2666) via NachoSoto (@NachoSoto)
* `Integration Tests`: added tests to verify 304 behavior (#2659) via
NachoSoto (@NachoSoto)
* `HTTPClient`: disable `URLSession` cache (#2668) via NachoSoto
(@NachoSoto)
* Documented `HTTPStatusCode.isSuccessfullySynced` (#2661) via NachoSoto
(@NachoSoto)
* `NetworkError.signatureVerificationFailed`: added status code to error
`userInfo` (#2657) via NachoSoto (@NachoSoto)
* `HTTPClient`: improved log for failed requests (#2669) via NachoSoto
(@NachoSoto)
* `ETagManager`: added new verbose logs (#2656) via NachoSoto
(@NachoSoto)
* `Signature Verification`: added test-only log for debugging invalid
signatures (#2658) via NachoSoto (@NachoSoto)
* Fixed `HTTPResponse.description` (#2664) via NachoSoto (@NachoSoto)
* Changed `Logger` to use `os_log` (#2608) via NachoSoto (@NachoSoto)
* `MainThreadMonitor`: increased threshold (#2662) via NachoSoto
(@NachoSoto)
* `debugRevenueCatOverlay`: display `receiptURL` (#2652) via NachoSoto
(@NachoSoto)
* `PurchaseTester`: added ability to display `debugRevenueCatOverlay`
(#2653) via NachoSoto (@NachoSoto)
* `debugRevenueCatOverlay`: ability to close on `macOS`/`Catalyst`
(#2649) via NachoSoto (@NachoSoto)
* `debugRevenueCatOverlay`: added support for `macOS` (#2648) via
NachoSoto (@NachoSoto)
* `LoadShedderIntegrationTests`: enable signature verification (#2655)
via NachoSoto (@NachoSoto)
* `ImageSnapshot`: fixed Xcode 15 compilation (#2651) via NachoSoto
(@NachoSoto)
* `OfferingsManager`: don't clear offerings cache timestamp when request
fails (#2359) via NachoSoto (@NachoSoto)
* `StoreKitObserverModeIntegrationTests`: added test for posting
renewals (#2590) via NachoSoto (@NachoSoto)
* Always initialize `StoreKit2TransactionListener` even on SK1 mode
(#2612) via NachoSoto (@NachoSoto)
* `ErrorUtils.missingReceiptFileError`: added receipt URL `userInfo`
context (#2650) via NachoSoto (@NachoSoto)
* Added `.xcprivacy` for Xcode 15 (#2619) via NachoSoto (@NachoSoto)
* `Trusted Entitlements`: added debug log with
`ResponseVerificationMode` (#2647) via NachoSoto (@NachoSoto)
* `debugRevenueCatOverlay`: simplified title (#2641) via NachoSoto
(@NachoSoto)
* Simplified `Purchases.updateAllCachesIfNeeded` (#2626) via NachoSoto
(@NachoSoto)
* `HTTPResponseTests`: fixed disabled test (#2643) via NachoSoto
(@NachoSoto)
* Add `InternalDangerousSettings.forceSignatureFailures` (#2635) via
NachoSoto (@NachoSoto)
* `IntegrationTests`: explicit `StoreKit 1` mode (#2636) via NachoSoto
(@NachoSoto)
* `Signing`: removed API for loading key from a file (#2638) via
NachoSoto (@NachoSoto)
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