Use uint64_t for fingerprints to prevent easy overflows Instead of using int for state/command/traits/components fingerprints, use uint64_t and reserve the value of 0 as a special "ignore" whildcard (instead of former -1). Change-Id: I3b95b4a8f9f41a963486d31ca6632ec0738dd7e9 Reviewed-on: https://weave-review.googlesource.com/1793 Reviewed-by: Alex Vakulenko <avakulenko@google.com>
diff --git a/src/privet/privet_handler.cc b/src/privet/privet_handler.cc index 9199c6f..0c9887f 100644 --- a/src/privet/privet_handler.cc +++ b/src/privet/privet_handler.cc
@@ -389,7 +389,7 @@ void PrivetHandler::OnTraitDefsChanged() { ++traits_fingerprint_; auto pred = [this](const UpdateRequestParameters& params) { - return params.traits_fingerprint < 0; + return params.traits_fingerprint == 0; }; auto last = std::partition(update_requests_.begin(), update_requests_.end(), pred); @@ -403,7 +403,7 @@ ++state_fingerprint_; ++components_fingerprint_; auto pred = [this](const UpdateRequestParameters& params) { - return params.state_fingerprint < 0 && params.components_fingerprint < 0; + return params.state_fingerprint == 0 && params.components_fingerprint == 0; }; auto last = std::partition(update_requests_.begin(), update_requests_.end(), pred); @@ -415,7 +415,7 @@ void PrivetHandler::OnComponentTreeChanged() { ++components_fingerprint_; auto pred = [this](const UpdateRequestParameters& params) { - return params.components_fingerprint < 0; + return params.components_fingerprint == 0; }; auto last = std::partition(update_requests_.begin(), update_requests_.end(), pred); @@ -930,10 +930,10 @@ params.request_id = ++last_update_request_id_; params.callback = callback; params.traits_fingerprint = - (ignore_traits && ignore_commands) ? -1 : traits_fingerprint_; - params.state_fingerprint = ignore_state ? -1 : state_fingerprint_; + (ignore_traits && ignore_commands) ? 0 : traits_fingerprint_; + params.state_fingerprint = ignore_state ? 0 : state_fingerprint_; params.components_fingerprint = - ignore_components ? -1 : components_fingerprint_; + ignore_components ? 0 : components_fingerprint_; update_requests_.push_back(params); if (timeout != base::TimeDelta::Max()) { device_->PostDelayedTask(
diff --git a/src/privet/privet_handler.h b/src/privet/privet_handler.h index 6bc4ac6..fc024d1 100644 --- a/src/privet/privet_handler.h +++ b/src/privet/privet_handler.h
@@ -130,10 +130,10 @@ void ReplyToUpdateRequest(const RequestCallback& callback) const; void OnUpdateRequestTimeout(int update_request_id); - CloudDelegate* cloud_ = nullptr; - DeviceDelegate* device_ = nullptr; - SecurityDelegate* security_ = nullptr; - WifiDelegate* wifi_ = nullptr; + CloudDelegate* cloud_{nullptr}; + DeviceDelegate* device_{nullptr}; + SecurityDelegate* security_{nullptr}; + WifiDelegate* wifi_{nullptr}; struct HandlerParameters { ApiHandler handler; @@ -144,18 +144,18 @@ struct UpdateRequestParameters { RequestCallback callback; - int request_id = 0; - int state_fingerprint = -1; - int traits_fingerprint = -1; - int components_fingerprint = -1; + int request_id{0}; + uint64_t state_fingerprint{0}; + uint64_t traits_fingerprint{0}; + uint64_t components_fingerprint{0}; }; std::vector<UpdateRequestParameters> update_requests_; int last_update_request_id_{0}; uint64_t last_user_id_{0}; - int state_fingerprint_{0}; - int traits_fingerprint_{0}; - int components_fingerprint_{0}; + uint64_t state_fingerprint_{1}; + uint64_t traits_fingerprint_{1}; + uint64_t components_fingerprint_{1}; ScopedObserver<CloudDelegate, CloudDelegate::Observer> cloud_observer_{this}; base::WeakPtrFactory<PrivetHandler> weak_ptr_factory_{this};
diff --git a/src/privet/privet_handler_unittest.cc b/src/privet/privet_handler_unittest.cc index 77cabdd..746fca9 100644 --- a/src/privet/privet_handler_unittest.cc +++ b/src/privet/privet_handler_unittest.cc
@@ -589,48 +589,48 @@ } TEST_F(PrivetHandlerSetupTest, State) { - EXPECT_JSON_EQ("{'state': {'test': {}}, 'fingerprint': '0'}", + EXPECT_JSON_EQ("{'state': {'test': {}}, 'fingerprint': '1'}", HandleRequest("/privet/v3/state", "{}")); cloud_.NotifyOnStateChanged(); - EXPECT_JSON_EQ("{'state': {'test': {}}, 'fingerprint': '1'}", + EXPECT_JSON_EQ("{'state': {'test': {}}, 'fingerprint': '2'}", HandleRequest("/privet/v3/state", "{}")); } TEST_F(PrivetHandlerSetupTest, CommandsDefs) { - EXPECT_JSON_EQ("{'commands': {'test':{}}, 'fingerprint': '0'}", + EXPECT_JSON_EQ("{'commands': {'test':{}}, 'fingerprint': '1'}", HandleRequest("/privet/v3/commandDefs", "{}")); cloud_.NotifyOnTraitDefsChanged(); - EXPECT_JSON_EQ("{'commands': {'test':{}}, 'fingerprint': '1'}", + EXPECT_JSON_EQ("{'commands': {'test':{}}, 'fingerprint': '2'}", HandleRequest("/privet/v3/commandDefs", "{}")); } TEST_F(PrivetHandlerSetupTest, Traits) { - EXPECT_JSON_EQ("{'traits': {'test': {}}, 'fingerprint': '0'}", + EXPECT_JSON_EQ("{'traits': {'test': {}}, 'fingerprint': '1'}", HandleRequest("/privet/v3/traits", "{}")); cloud_.NotifyOnTraitDefsChanged(); - EXPECT_JSON_EQ("{'traits': {'test': {}}, 'fingerprint': '1'}", + EXPECT_JSON_EQ("{'traits': {'test': {}}, 'fingerprint': '2'}", HandleRequest("/privet/v3/traits", "{}")); } TEST_F(PrivetHandlerSetupTest, Components) { - EXPECT_JSON_EQ("{'components': {'test': {}}, 'fingerprint': '0'}", + EXPECT_JSON_EQ("{'components': {'test': {}}, 'fingerprint': '1'}", HandleRequest("/privet/v3/components", "{}")); cloud_.NotifyOnComponentTreeChanged(); - EXPECT_JSON_EQ("{'components': {'test': {}}, 'fingerprint': '1'}", + EXPECT_JSON_EQ("{'components': {'test': {}}, 'fingerprint': '2'}", HandleRequest("/privet/v3/components", "{}")); // State change will also change the components fingerprint. cloud_.NotifyOnStateChanged(); - EXPECT_JSON_EQ("{'components': {'test': {}}, 'fingerprint': '2'}", + EXPECT_JSON_EQ("{'components': {'test': {}}, 'fingerprint': '3'}", HandleRequest("/privet/v3/components", "{}")); } @@ -727,10 +727,10 @@ cloud_.NotifyOnStateChanged(); const char kInput[] = "{}"; const char kExpected[] = R"({ - 'commandsFingerprint': '1', - 'stateFingerprint': '1', - 'traitsFingerprint': '1', - 'componentsFingerprint': '2' + 'commandsFingerprint': '2', + 'stateFingerprint': '2', + 'traitsFingerprint': '2', + 'componentsFingerprint': '3' })"; EXPECT_JSON_EQ(kExpected, HandleRequest("/privet/v3/checkForUpdates", kInput)); @@ -744,16 +744,16 @@ cloud_.NotifyOnComponentTreeChanged(); cloud_.NotifyOnStateChanged(); const char kInput[] = R"({ - 'commandsFingerprint': '0', - 'stateFingerprint': '0', - 'traitsFingerprint': '0', - 'componentsFingerprint': '0' - })"; - const char kExpected[] = R"({ 'commandsFingerprint': '1', 'stateFingerprint': '1', 'traitsFingerprint': '1', - 'componentsFingerprint': '2' + 'componentsFingerprint': '1' + })"; + const char kExpected[] = R"({ + 'commandsFingerprint': '2', + 'stateFingerprint': '2', + 'traitsFingerprint': '2', + 'componentsFingerprint': '3' })"; EXPECT_JSON_EQ(kExpected, HandleRequest("/privet/v3/checkForUpdates", kInput)); @@ -764,20 +764,20 @@ EXPECT_CALL(device_, GetHttpRequestTimeout()) .WillOnce(Return(base::TimeDelta::Max())); const char kInput[] = R"({ - 'commandsFingerprint': '0', - 'stateFingerprint': '0', - 'traitsFingerprint': '0', - 'componentsFingerprint': '0' + 'commandsFingerprint': '1', + 'stateFingerprint': '1', + 'traitsFingerprint': '1', + 'componentsFingerprint': '1' })"; EXPECT_JSON_EQ("{}", HandleRequest("/privet/v3/checkForUpdates", kInput)); EXPECT_EQ(0, GetResponseCount()); cloud_.NotifyOnTraitDefsChanged(); EXPECT_EQ(1, GetResponseCount()); const char kExpected[] = R"({ - 'commandsFingerprint': '1', - 'stateFingerprint': '0', - 'traitsFingerprint': '1', - 'componentsFingerprint': '0' + 'commandsFingerprint': '2', + 'stateFingerprint': '1', + 'traitsFingerprint': '2', + 'componentsFingerprint': '1' })"; EXPECT_JSON_EQ(kExpected, GetResponse()); } @@ -786,20 +786,20 @@ EXPECT_CALL(device_, GetHttpRequestTimeout()) .WillOnce(Return(base::TimeDelta::Max())); const char kInput[] = R"({ - 'commandsFingerprint': '0', - 'stateFingerprint': '0', - 'traitsFingerprint': '0', - 'componentsFingerprint': '0' + 'commandsFingerprint': '1', + 'stateFingerprint': '1', + 'traitsFingerprint': '1', + 'componentsFingerprint': '1' })"; EXPECT_JSON_EQ("{}", HandleRequest("/privet/v3/checkForUpdates", kInput)); EXPECT_EQ(0, GetResponseCount()); cloud_.NotifyOnTraitDefsChanged(); EXPECT_EQ(1, GetResponseCount()); const char kExpected[] = R"({ - 'commandsFingerprint': '1', - 'stateFingerprint': '0', - 'traitsFingerprint': '1', - 'componentsFingerprint': '0' + 'commandsFingerprint': '2', + 'stateFingerprint': '1', + 'traitsFingerprint': '2', + 'componentsFingerprint': '1' })"; EXPECT_JSON_EQ(kExpected, GetResponse()); } @@ -808,20 +808,20 @@ EXPECT_CALL(device_, GetHttpRequestTimeout()) .WillOnce(Return(base::TimeDelta::Max())); const char kInput[] = R"({ - 'commandsFingerprint': '0', - 'stateFingerprint': '0', - 'traitsFingerprint': '0', - 'componentsFingerprint': '0' + 'commandsFingerprint': '1', + 'stateFingerprint': '1', + 'traitsFingerprint': '1', + 'componentsFingerprint': '1' })"; EXPECT_JSON_EQ("{}", HandleRequest("/privet/v3/checkForUpdates", kInput)); EXPECT_EQ(0, GetResponseCount()); cloud_.NotifyOnStateChanged(); EXPECT_EQ(1, GetResponseCount()); const char kExpected[] = R"({ - 'commandsFingerprint': '0', - 'stateFingerprint': '1', - 'traitsFingerprint': '0', - 'componentsFingerprint': '1' + 'commandsFingerprint': '1', + 'stateFingerprint': '2', + 'traitsFingerprint': '1', + 'componentsFingerprint': '2' })"; EXPECT_JSON_EQ(kExpected, GetResponse()); } @@ -830,60 +830,14 @@ EXPECT_CALL(device_, GetHttpRequestTimeout()) .WillOnce(Return(base::TimeDelta::Max())); const char kInput[] = R"({ - 'commandsFingerprint': '0', - 'stateFingerprint': '0', - 'traitsFingerprint': '0', - 'componentsFingerprint': '0' - })"; - EXPECT_JSON_EQ("{}", HandleRequest("/privet/v3/checkForUpdates", kInput)); - EXPECT_EQ(0, GetResponseCount()); - cloud_.NotifyOnComponentTreeChanged(); - EXPECT_EQ(1, GetResponseCount()); - const char kExpected[] = R"({ - 'commandsFingerprint': '0', - 'stateFingerprint': '0', - 'traitsFingerprint': '0', - 'componentsFingerprint': '1' - })"; - EXPECT_JSON_EQ(kExpected, GetResponse()); -} - -TEST_F(PrivetHandlerSetupTest, CheckForUpdates_LongPollIgnoreTraits) { - EXPECT_CALL(device_, GetHttpRequestTimeout()) - .WillOnce(Return(base::TimeDelta::Max())); - const char kInput[] = R"({ - 'stateFingerprint': '0', - 'componentsFingerprint': '0' - })"; - EXPECT_JSON_EQ("{}", HandleRequest("/privet/v3/checkForUpdates", kInput)); - EXPECT_EQ(0, GetResponseCount()); - cloud_.NotifyOnTraitDefsChanged(); - EXPECT_EQ(0, GetResponseCount()); - cloud_.NotifyOnComponentTreeChanged(); - EXPECT_EQ(1, GetResponseCount()); - const char kExpected[] = R"({ 'commandsFingerprint': '1', - 'stateFingerprint': '0', + 'stateFingerprint': '1', 'traitsFingerprint': '1', 'componentsFingerprint': '1' })"; - EXPECT_JSON_EQ(kExpected, GetResponse()); -} - -TEST_F(PrivetHandlerSetupTest, CheckForUpdates_LongPollIgnoreState) { - EXPECT_CALL(device_, GetHttpRequestTimeout()) - .WillOnce(Return(base::TimeDelta::Max())); - const char kInput[] = R"({ - 'commandsFingerprint': '0', - 'traitsFingerprint': '0' - })"; EXPECT_JSON_EQ("{}", HandleRequest("/privet/v3/checkForUpdates", kInput)); EXPECT_EQ(0, GetResponseCount()); - cloud_.NotifyOnStateChanged(); - EXPECT_EQ(0, GetResponseCount()); cloud_.NotifyOnComponentTreeChanged(); - EXPECT_EQ(0, GetResponseCount()); - cloud_.NotifyOnTraitDefsChanged(); EXPECT_EQ(1, GetResponseCount()); const char kExpected[] = R"({ 'commandsFingerprint': '1', @@ -894,21 +848,67 @@ EXPECT_JSON_EQ(kExpected, GetResponse()); } +TEST_F(PrivetHandlerSetupTest, CheckForUpdates_LongPollIgnoreTraits) { + EXPECT_CALL(device_, GetHttpRequestTimeout()) + .WillOnce(Return(base::TimeDelta::Max())); + const char kInput[] = R"({ + 'stateFingerprint': '1', + 'componentsFingerprint': '1' + })"; + EXPECT_JSON_EQ("{}", HandleRequest("/privet/v3/checkForUpdates", kInput)); + EXPECT_EQ(0, GetResponseCount()); + cloud_.NotifyOnTraitDefsChanged(); + EXPECT_EQ(0, GetResponseCount()); + cloud_.NotifyOnComponentTreeChanged(); + EXPECT_EQ(1, GetResponseCount()); + const char kExpected[] = R"({ + 'commandsFingerprint': '2', + 'stateFingerprint': '1', + 'traitsFingerprint': '2', + 'componentsFingerprint': '2' + })"; + EXPECT_JSON_EQ(kExpected, GetResponse()); +} + +TEST_F(PrivetHandlerSetupTest, CheckForUpdates_LongPollIgnoreState) { + EXPECT_CALL(device_, GetHttpRequestTimeout()) + .WillOnce(Return(base::TimeDelta::Max())); + const char kInput[] = R"({ + 'commandsFingerprint': '1', + 'traitsFingerprint': '1' + })"; + EXPECT_JSON_EQ("{}", HandleRequest("/privet/v3/checkForUpdates", kInput)); + EXPECT_EQ(0, GetResponseCount()); + cloud_.NotifyOnStateChanged(); + EXPECT_EQ(0, GetResponseCount()); + cloud_.NotifyOnComponentTreeChanged(); + EXPECT_EQ(0, GetResponseCount()); + cloud_.NotifyOnTraitDefsChanged(); + EXPECT_EQ(1, GetResponseCount()); + const char kExpected[] = R"({ + 'commandsFingerprint': '2', + 'stateFingerprint': '2', + 'traitsFingerprint': '2', + 'componentsFingerprint': '3' + })"; + EXPECT_JSON_EQ(kExpected, GetResponse()); +} + TEST_F(PrivetHandlerSetupTest, CheckForUpdates_InstantTimeout) { EXPECT_CALL(device_, GetHttpRequestTimeout()) .WillOnce(Return(base::TimeDelta::Max())); const char kInput[] = R"({ - 'commandsFingerprint': '0', - 'stateFingerprint': '0', - 'traitsFingerprint': '0', - 'componentsFingerprint': '0', + 'commandsFingerprint': '1', + 'stateFingerprint': '1', + 'traitsFingerprint': '1', + 'componentsFingerprint': '1', 'waitTimeout': 0 })"; const char kExpected[] = R"({ - 'commandsFingerprint': '0', - 'stateFingerprint': '0', - 'traitsFingerprint': '0', - 'componentsFingerprint': '0' + 'commandsFingerprint': '1', + 'stateFingerprint': '1', + 'traitsFingerprint': '1', + 'componentsFingerprint': '1' })"; EXPECT_JSON_EQ(kExpected, HandleRequest("/privet/v3/checkForUpdates", kInput)); @@ -918,10 +918,10 @@ EXPECT_CALL(device_, GetHttpRequestTimeout()) .WillOnce(Return(base::TimeDelta::Max())); const char kInput[] = R"({ - 'commandsFingerprint': '0', - 'stateFingerprint': '0', - 'traitsFingerprint': '0', - 'componentsFingerprint': '0', + 'commandsFingerprint': '1', + 'stateFingerprint': '1', + 'traitsFingerprint': '1', + 'componentsFingerprint': '1', 'waitTimeout': 3 })"; base::Closure callback; @@ -932,10 +932,10 @@ callback.Run(); EXPECT_EQ(1, GetResponseCount()); const char kExpected[] = R"({ - 'commandsFingerprint': '0', - 'stateFingerprint': '0', - 'traitsFingerprint': '0', - 'componentsFingerprint': '0' + 'commandsFingerprint': '1', + 'stateFingerprint': '1', + 'traitsFingerprint': '1', + 'componentsFingerprint': '1' })"; EXPECT_JSON_EQ(kExpected, GetResponse()); } @@ -944,10 +944,10 @@ EXPECT_CALL(device_, GetHttpRequestTimeout()) .WillOnce(Return(base::TimeDelta::FromMinutes(1))); const char kInput[] = R"({ - 'commandsFingerprint': '0', - 'stateFingerprint': '0', - 'traitsFingerprint': '0', - 'componentsFingerprint': '0' + 'commandsFingerprint': '1', + 'stateFingerprint': '1', + 'traitsFingerprint': '1', + 'componentsFingerprint': '1' })"; base::Closure callback; EXPECT_CALL(device_, PostDelayedTask(_, _, base::TimeDelta::FromSeconds(50))) @@ -957,10 +957,10 @@ callback.Run(); EXPECT_EQ(1, GetResponseCount()); const char kExpected[] = R"({ - 'commandsFingerprint': '0', - 'stateFingerprint': '0', - 'traitsFingerprint': '0', - 'componentsFingerprint': '0' + 'commandsFingerprint': '1', + 'stateFingerprint': '1', + 'traitsFingerprint': '1', + 'componentsFingerprint': '1' })"; EXPECT_JSON_EQ(kExpected, GetResponse()); } @@ -969,10 +969,10 @@ EXPECT_CALL(device_, GetHttpRequestTimeout()) .WillOnce(Return(base::TimeDelta::FromSeconds(5))); const char kInput[] = R"({ - 'commandsFingerprint': '0', - 'stateFingerprint': '0', - 'traitsFingerprint': '0', - 'componentsFingerprint': '0' + 'commandsFingerprint': '1', + 'stateFingerprint': '1', + 'traitsFingerprint': '1', + 'componentsFingerprint': '1' })"; EXPECT_JSON_EQ(kInput, HandleRequest("/privet/v3/checkForUpdates", kInput)); EXPECT_EQ(1, GetResponseCount()); @@ -982,10 +982,10 @@ EXPECT_CALL(device_, GetHttpRequestTimeout()) .WillOnce(Return(base::TimeDelta::FromMinutes(1))); const char kInput[] = R"({ - 'commandsFingerprint': '0', - 'stateFingerprint': '0', - 'traitsFingerprint': '0', - 'componentsFingerprint': '0', + 'commandsFingerprint': '1', + 'stateFingerprint': '1', + 'traitsFingerprint': '1', + 'componentsFingerprint': '1', 'waitTimeout': 10 })"; base::Closure callback; @@ -996,10 +996,10 @@ callback.Run(); EXPECT_EQ(1, GetResponseCount()); const char kExpected[] = R"({ - 'commandsFingerprint': '0', - 'stateFingerprint': '0', - 'traitsFingerprint': '0', - 'componentsFingerprint': '0' + 'commandsFingerprint': '1', + 'stateFingerprint': '1', + 'traitsFingerprint': '1', + 'componentsFingerprint': '1' })"; EXPECT_JSON_EQ(kExpected, GetResponse()); } @@ -1008,10 +1008,10 @@ EXPECT_CALL(device_, GetHttpRequestTimeout()) .WillOnce(Return(base::TimeDelta::Max())); const char kInput[] = R"({ - 'commandsFingerprint': '0', - 'stateFingerprint': '0', - 'traitsFingerprint': '0', - 'componentsFingerprint': '0', + 'commandsFingerprint': '1', + 'stateFingerprint': '1', + 'traitsFingerprint': '1', + 'componentsFingerprint': '1', 'waitTimeout': 10 })"; base::Closure callback; @@ -1022,10 +1022,10 @@ cloud_.NotifyOnTraitDefsChanged(); EXPECT_EQ(1, GetResponseCount()); const char kExpected[] = R"({ - 'commandsFingerprint': '1', - 'stateFingerprint': '0', - 'traitsFingerprint': '1', - 'componentsFingerprint': '0' + 'commandsFingerprint': '2', + 'stateFingerprint': '1', + 'traitsFingerprint': '2', + 'componentsFingerprint': '1' })"; EXPECT_JSON_EQ(kExpected, GetResponse()); callback.Run();