Skip to content

Commit 2cab34a

Browse files
committed
Add validity tests to cover edge cases
I added these tests to improve Go coverage for optimizations I want to be sure are safe. In adding these, I found two cases where the libraries of various language handle validation logic inconsistently. In cases of inconsistency, I defaulted to the behavior used by most language in the repository. 1. Automatically expanding requested short code length if odd. Most of the language implementations automatically add 1 to the length of requested codes if the length indicates it is in the pair part of the code. 2. Validate latitude and longitude bounds within validation logic for full codes. Most of the language implementations only check the first two characters for latitude/longitude bounds when within the validation logic for full codes, not when generally checking if codes are valid. This does introduce a strange case where a code is "valid", but is neither a valid short nor a valid long code. As this was the preexisting behavior, I have defaulted to this.
1 parent aec67de commit 2cab34a

File tree

4 files changed

+30
-27
lines changed

4 files changed

+30
-27
lines changed

cpp/openlocationcode.cc

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ std::string encodeIntegers(int64_t lat_val, int64_t lng_val,
7878
// Add the separator character.
7979
code[internal::kSeparatorPosition] = internal::kSeparator;
8080

81+
// Limit the maximum number of digits in the code.
82+
code_length = std::min(code_length, internal::kMaximumDigitCount);
83+
// Ensure the length is valid.
84+
code_length = std::max(code_length, internal::kMinimumDigitCount);
8185
// Compute the grid part of the code if necessary.
8286
if (code_length > internal::kPairCodeLength) {
8387
for (size_t i = internal::kGridCodeLength; i >= 1; i--) {
@@ -89,6 +93,9 @@ std::string encodeIntegers(int64_t lat_val, int64_t lng_val,
8993
lng_val /= internal::kGridColumns;
9094
}
9195
} else {
96+
if (code_length % 2 == 1) {
97+
code_length++;
98+
}
9299
lat_val /= pow(internal::kGridRows, internal::kGridCodeLength);
93100
lng_val /= pow(internal::kGridColumns, internal::kGridCodeLength);
94101
}
@@ -193,13 +200,6 @@ std::string clean_code_chars(const std::string &code) {
193200
} // anonymous namespace
194201

195202
std::string Encode(const LatLng &location, size_t code_length) {
196-
// Limit the maximum number of digits in the code.
197-
code_length = std::min(code_length, internal::kMaximumDigitCount);
198-
// Ensure the length is valid.
199-
code_length = std::max(code_length, internal::kMinimumDigitCount);
200-
if (code_length < internal::kPairCodeLength && code_length % 2 == 1) {
201-
code_length = code_length + 1;
202-
}
203203
// Convert latitude and longitude into integer values.
204204
int64_t lat_val = internal::latitudeToInteger(location.latitude);
205205
int64_t lng_val = internal::longitudeToInteger(location.longitude);

java/src/main/java/com/google/openlocationcode/OpenLocationCode.java

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,12 @@ public OpenLocationCode(double latitude, double longitude, int codeLength) {
205205
static String encodeIntegers(long lat, long lng, int codeLength) {
206206
// Limit the maximum number of digits in the code.
207207
codeLength = Math.min(codeLength, MAX_DIGIT_COUNT);
208+
// Automatically increase code length if odd and below pair code length.
209+
if ((codeLength < PAIR_CODE_LENGTH) && codeLength % 2 == 1) {
210+
codeLength += 1;
211+
}
208212
// Check that the code length requested is valid.
209-
if (codeLength < PAIR_CODE_LENGTH && codeLength % 2 == 1 || codeLength < MIN_DIGIT_COUNT) {
213+
if (codeLength < MIN_DIGIT_COUNT) {
210214
throw new IllegalArgumentException("Illegal code length " + codeLength);
211215
}
212216

@@ -359,6 +363,15 @@ public static CodeArea decode(String code) throws IllegalArgumentException {
359363
* @return True if it is a full code.
360364
*/
361365
public boolean isFull() {
366+
// First latitude character can only have first 9 values.
367+
if (CODE_ALPHABET.indexOf(code.charAt(0)) > 8) {
368+
return false;
369+
}
370+
371+
// First longitude character can only have first 18 values.
372+
if (CODE_ALPHABET.indexOf(code.charAt(1)) > 17) {
373+
return false;
374+
}
362375
return code.indexOf(SEPARATOR) == SEPARATOR_POSITION;
363376
}
364377

@@ -569,19 +582,6 @@ public static boolean isValidCode(String code) {
569582
return false;
570583
}
571584

572-
// Check first two characters: only some values from the alphabet are permitted.
573-
if (separatorPosition == SEPARATOR_POSITION) {
574-
// First latitude character can only have first 9 values.
575-
if (CODE_ALPHABET.indexOf(code.charAt(0)) > 8) {
576-
return false;
577-
}
578-
579-
// First longitude character can only have first 18 values.
580-
if (CODE_ALPHABET.indexOf(code.charAt(1)) > 17) {
581-
return false;
582-
}
583-
}
584-
585585
// Check the characters before the separator.
586586
boolean paddingStarted = false;
587587
for (int i = 0; i < separatorPosition; i++) {

js/closure/openlocationcode.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -420,10 +420,11 @@ function _encodeIntegers(latInt, lngInt, codeLength) {
420420
if (typeof codeLength == 'undefined') {
421421
codeLength = PAIR_CODE_LENGTH;
422422
}
423-
if (
424-
codeLength < MIN_CODE_LEN ||
425-
(codeLength < PAIR_CODE_LENGTH && codeLength % 2 == 1)
426-
) {
423+
// Increment length if short and not even.
424+
if (codeLength < PAIR_CODE_LENGTH && codeLength % 2 === 1) {
425+
codeLength++;
426+
}
427+
if (codeLength < MIN_CODE_LEN) {
427428
throw new Error('IllegalArgumentException: Invalid Plus Code length');
428429
}
429430
codeLength = Math.min(codeLength, MAX_CODE_LEN);

python/openlocationcode/openlocationcode.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,10 @@ def encodeIntegers(latVal, lngVal, codeLength):
287287
This function is exposed for testing purposes and should not be called
288288
directly.
289289
"""
290-
if codeLength < MIN_DIGIT_COUNT_ or (codeLength < PAIR_CODE_LENGTH_ and
291-
codeLength % 2 == 1):
290+
if codeLength < PAIR_CODE_LENGTH_ and codeLength % 2 == 1:
291+
# Automatically increment short lengths to be even.
292+
codeLength+=1
293+
if codeLength < MIN_DIGIT_COUNT_:
292294
raise ValueError('Invalid Open Location Code length - ' +
293295
str(codeLength))
294296
codeLength = min(codeLength, MAX_DIGIT_COUNT_)

0 commit comments

Comments
 (0)