From 7eb8ce2ad376f49f5f6c65d128036615b4de98d6 Mon Sep 17 00:00:00 2001 From: cristi-lupu Date: Tue, 7 Apr 2020 17:02:45 +0300 Subject: [PATCH 1/4] Implement maximum request size --- Analytics/Classes/Internal/SEGHTTPClient.h | 2 +- Analytics/Classes/Internal/SEGHTTPClient.m | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Analytics/Classes/Internal/SEGHTTPClient.h b/Analytics/Classes/Internal/SEGHTTPClient.h index 284133f88..59a6897cd 100644 --- a/Analytics/Classes/Internal/SEGHTTPClient.h +++ b/Analytics/Classes/Internal/SEGHTTPClient.h @@ -33,7 +33,7 @@ NS_ASSUME_NONNULL_BEGIN * NOTE: You need to re-dispatch within the completionHandler onto a desired queue to avoid threading issues. * Completion handlers are called on a dispatch queue internal to SEGHTTPClient. */ -- (NSURLSessionUploadTask *)upload:(JSON_DICT)batch forWriteKey:(NSString *)writeKey completionHandler:(void (^)(BOOL retry))completionHandler; +- (nullable NSURLSessionUploadTask *)upload:(JSON_DICT)batch forWriteKey:(NSString *)writeKey completionHandler:(void (^)(BOOL retry))completionHandler; - (NSURLSessionDataTask *)settingsForWriteKey:(NSString *)writeKey completionHandler:(void (^)(BOOL success, JSON_DICT _Nullable settings))completionHandler; diff --git a/Analytics/Classes/Internal/SEGHTTPClient.m b/Analytics/Classes/Internal/SEGHTTPClient.m index 21c24bd20..04c25bb8c 100644 --- a/Analytics/Classes/Internal/SEGHTTPClient.m +++ b/Analytics/Classes/Internal/SEGHTTPClient.m @@ -2,6 +2,7 @@ #import "NSData+SEGGZIP.h" #import "SEGAnalyticsUtils.h" +static const NSUInteger kMaxBatchSize = 500000; // 500KB @implementation SEGHTTPClient @@ -66,7 +67,7 @@ - (void)dealloc } -- (NSURLSessionUploadTask *)upload:(NSDictionary *)batch forWriteKey:(NSString *)writeKey completionHandler:(void (^)(BOOL retry))completionHandler +- (nullable NSURLSessionUploadTask *)upload:(NSDictionary *)batch forWriteKey:(NSString *)writeKey completionHandler:(void (^)(BOOL retry))completionHandler { // batch = SEGCoerceDictionary(batch); NSURLSession *session = [self sessionForWriteKey:writeKey]; @@ -94,6 +95,11 @@ - (NSURLSessionUploadTask *)upload:(NSDictionary *)batch forWriteKey:(NSString * return nil; } NSData *gzippedPayload = [payload seg_gzippedData]; + if (gzippedPayload.length >= kMaxBatchSize) { + SEGLog(@"Payload exceeded the limit of %luKB per batch", kMaxBatchSize / 1000); + completionHandler(NO); + return nil; + } NSURLSessionUploadTask *task = [session uploadTaskWithRequest:request fromData:gzippedPayload completionHandler:^(NSData *_Nullable data, NSURLResponse *_Nullable response, NSError *_Nullable error) { if (error) { From 86ec624b9c9ec7e24a0c932cab33fa5c118428a4 Mon Sep 17 00:00:00 2001 From: cristi-lupu Date: Tue, 7 Apr 2020 17:03:19 +0300 Subject: [PATCH 2/4] Add test when batch exceeds the size --- AnalyticsTests/HTTPClientTest.swift | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/AnalyticsTests/HTTPClientTest.swift b/AnalyticsTests/HTTPClientTest.swift index c57350627..d3e70ae01 100644 --- a/AnalyticsTests/HTTPClientTest.swift +++ b/AnalyticsTests/HTTPClientTest.swift @@ -127,7 +127,7 @@ class HTTPClientTest: QuickSpec { done = true } expect(done).toEventually(beTrue()) - expect(task.state).toEventually(equal(URLSessionTask.State.completed)) + expect(task?.state).toEventually(equal(URLSessionTask.State.completed)) } it("asks to retry for 3xx response") { @@ -142,7 +142,7 @@ class HTTPClientTest: QuickSpec { done = true } expect(done).toEventually(beTrue()) - expect(task.state).toEventually(equal(URLSessionTask.State.completed)) + expect(task?.state).toEventually(equal(URLSessionTask.State.completed)) } it("does not ask to retry for 4xx response") { @@ -157,7 +157,7 @@ class HTTPClientTest: QuickSpec { done = true } expect(done).toEventually(beTrue()) - expect(task.state).toEventually(equal(URLSessionTask.State.completed)) + expect(task?.state).toEventually(equal(URLSessionTask.State.completed)) } it("asks to retry for 429 response") { @@ -172,7 +172,7 @@ class HTTPClientTest: QuickSpec { done = true } expect(done).toEventually(beTrue()) - expect(task.state).toEventually(equal(URLSessionTask.State.completed)) + expect(task?.state).toEventually(equal(URLSessionTask.State.completed)) } it("asks to retry for 5xx response") { @@ -187,7 +187,24 @@ class HTTPClientTest: QuickSpec { done = true } expect(done).toEventually(beTrue()) - expect(task.state).toEventually(equal(URLSessionTask.State.completed)) + expect(task?.state).toEventually(equal(URLSessionTask.State.completed)) + } + + it("fails when batch size exceeds the max limit size") { + let oversizedBatch: [String: Any] = ["sentAt":"2016-07-19'T'19:25:06Z", + "batch": Array(repeating: ["type":"track", "event":"foo"], count: 7000000)] + _ = stubRequest("POST", "https://api.segment.io/v1/batch" as NSString) + .withHeader("User-Agent", "analytics-ios/" + SEGAnalytics.version())! + .withJsonGzippedBody(oversizedBatch as AnyObject) + .withWriteKey("bar") + .andReturn(429) + var done = false + let task = client.upload(oversizedBatch, forWriteKey: "bar") { retry in + expect(retry) == false + done = true + } + expect(done).toEventually(beTrue()) + expect(task).toEventually(beNil()) } } From 09a244bae8a2365348d03ab263cea7b5325680ba Mon Sep 17 00:00:00 2001 From: cristi-lupu Date: Fri, 10 Apr 2020 12:41:50 +0300 Subject: [PATCH 3/4] Execute check before gzip --- Analytics/Classes/Internal/SEGHTTPClient.m | 6 +++--- AnalyticsTests/HTTPClientTest.swift | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Analytics/Classes/Internal/SEGHTTPClient.m b/Analytics/Classes/Internal/SEGHTTPClient.m index 04c25bb8c..d89b2fd27 100644 --- a/Analytics/Classes/Internal/SEGHTTPClient.m +++ b/Analytics/Classes/Internal/SEGHTTPClient.m @@ -2,7 +2,7 @@ #import "NSData+SEGGZIP.h" #import "SEGAnalyticsUtils.h" -static const NSUInteger kMaxBatchSize = 500000; // 500KB +static const NSUInteger kMaxBatchSize = 475000; // 475KB @implementation SEGHTTPClient @@ -94,12 +94,12 @@ - (nullable NSURLSessionUploadTask *)upload:(NSDictionary *)batch forWriteKey:(N completionHandler(NO); // Don't retry this batch. return nil; } - NSData *gzippedPayload = [payload seg_gzippedData]; - if (gzippedPayload.length >= kMaxBatchSize) { + if (payload.length >= kMaxBatchSize) { SEGLog(@"Payload exceeded the limit of %luKB per batch", kMaxBatchSize / 1000); completionHandler(NO); return nil; } + NSData *gzippedPayload = [payload seg_gzippedData]; NSURLSessionUploadTask *task = [session uploadTaskWithRequest:request fromData:gzippedPayload completionHandler:^(NSData *_Nullable data, NSURLResponse *_Nullable response, NSError *_Nullable error) { if (error) { diff --git a/AnalyticsTests/HTTPClientTest.swift b/AnalyticsTests/HTTPClientTest.swift index d3e70ae01..2675dcc3e 100644 --- a/AnalyticsTests/HTTPClientTest.swift +++ b/AnalyticsTests/HTTPClientTest.swift @@ -192,7 +192,7 @@ class HTTPClientTest: QuickSpec { it("fails when batch size exceeds the max limit size") { let oversizedBatch: [String: Any] = ["sentAt":"2016-07-19'T'19:25:06Z", - "batch": Array(repeating: ["type":"track", "event":"foo"], count: 7000000)] + "batch": Array(repeating: ["type":"track", "event":"foo"], count: 16000)] _ = stubRequest("POST", "https://api.segment.io/v1/batch" as NSString) .withHeader("User-Agent", "analytics-ios/" + SEGAnalytics.version())! .withJsonGzippedBody(oversizedBatch as AnyObject) From 02e7258ff2099108f38e7198ff80b0ab0dedffc4 Mon Sep 17 00:00:00 2001 From: cristi-lupu Date: Fri, 10 Apr 2020 12:52:07 +0300 Subject: [PATCH 4/4] Remove stub request --- AnalyticsTests/HTTPClientTest.swift | 5 ----- 1 file changed, 5 deletions(-) diff --git a/AnalyticsTests/HTTPClientTest.swift b/AnalyticsTests/HTTPClientTest.swift index 2675dcc3e..d25c2b696 100644 --- a/AnalyticsTests/HTTPClientTest.swift +++ b/AnalyticsTests/HTTPClientTest.swift @@ -193,11 +193,6 @@ class HTTPClientTest: QuickSpec { it("fails when batch size exceeds the max limit size") { let oversizedBatch: [String: Any] = ["sentAt":"2016-07-19'T'19:25:06Z", "batch": Array(repeating: ["type":"track", "event":"foo"], count: 16000)] - _ = stubRequest("POST", "https://api.segment.io/v1/batch" as NSString) - .withHeader("User-Agent", "analytics-ios/" + SEGAnalytics.version())! - .withJsonGzippedBody(oversizedBatch as AnyObject) - .withWriteKey("bar") - .andReturn(429) var done = false let task = client.upload(oversizedBatch, forWriteKey: "bar") { retry in expect(retry) == false