From b09db1bf141b8ddb29ab60c66f044f7519a3b3de Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Fri, 1 Nov 2019 16:08:03 -0700 Subject: [PATCH 1/5] Handle new style URLs, and preserve the old behavior when needed. --- packages/google_sign_in/google_sign_in/CHANGELOG.md | 4 ++++ .../google_sign_in/google_sign_in/lib/widgets.dart | 12 +++++++++--- packages/google_sign_in/google_sign_in/pubspec.yaml | 2 +- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/google_sign_in/google_sign_in/CHANGELOG.md b/packages/google_sign_in/google_sign_in/CHANGELOG.md index a180ba8e1a3f..77786459b7f5 100644 --- a/packages/google_sign_in/google_sign_in/CHANGELOG.md +++ b/packages/google_sign_in/google_sign_in/CHANGELOG.md @@ -1,3 +1,7 @@ +## 4.0.13 + +* Fix GoogleUserCircleAvatar to handle new style profile image URLs. + ## 4.0.12 * Move google_sign_in plugin to google_sign_in/google_sign_in to prepare for federated implementations. diff --git a/packages/google_sign_in/google_sign_in/lib/widgets.dart b/packages/google_sign_in/google_sign_in/lib/widgets.dart index 01ab6c64c00c..7011c05b9e79 100644 --- a/packages/google_sign_in/google_sign_in/lib/widgets.dart +++ b/packages/google_sign_in/google_sign_in/lib/widgets.dart @@ -80,9 +80,15 @@ class GoogleUserCircleAvatar extends StatelessWidget { final Uri profileUri = Uri.parse(photoUrl); final List pathSegments = List.from(profileUri.pathSegments); - pathSegments - ..removeWhere(sizeDirective.hasMatch) - ..insert(pathSegments.length - 1, 's${size.round()}-c'); + if (pathSegments.length <= 2) { + // New style URLs + pathSegments.last = '${pathSegments.last}=s${size.round()}-c'; + } else { + // Old style URLs + pathSegments + ..removeWhere(sizeDirective.hasMatch) + ..insert(pathSegments.length - 1, 's${size.round()}-c'); + } return Uri( scheme: profileUri.scheme, host: profileUri.host, diff --git a/packages/google_sign_in/google_sign_in/pubspec.yaml b/packages/google_sign_in/google_sign_in/pubspec.yaml index d3331d24f55d..833ef6733d2d 100644 --- a/packages/google_sign_in/google_sign_in/pubspec.yaml +++ b/packages/google_sign_in/google_sign_in/pubspec.yaml @@ -3,7 +3,7 @@ description: Flutter plugin for Google Sign-In, a secure authentication system for signing in with a Google account on Android and iOS. author: Flutter Team homepage: https://github.com/flutter/plugins/tree/master/packages/google_sign_in/google_sign_in -version: 4.0.12 +version: 4.0.13 flutter: plugin: From 5c5ac55cb463cbc4c94cd0341e1e6d4560f8e887 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Fri, 8 Nov 2019 13:08:33 -0800 Subject: [PATCH 2/5] Extract URL manipulation to its own file, and add unit tests. --- .../google_sign_in/lib/widgets.dart | 24 ++----- packages/google_sign_in/lib/src/fife.dart | 53 +++++++++++++++ packages/google_sign_in/test/fife.dart | 66 +++++++++++++++++++ 3 files changed, 123 insertions(+), 20 deletions(-) create mode 100644 packages/google_sign_in/lib/src/fife.dart create mode 100644 packages/google_sign_in/test/fife.dart diff --git a/packages/google_sign_in/google_sign_in/lib/widgets.dart b/packages/google_sign_in/google_sign_in/lib/widgets.dart index 7011c05b9e79..3375628f47b5 100644 --- a/packages/google_sign_in/google_sign_in/lib/widgets.dart +++ b/packages/google_sign_in/google_sign_in/lib/widgets.dart @@ -8,6 +8,7 @@ import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'src/common.dart'; +import 'src/fife.dart' as fife; /// Builds a CircleAvatar profile image of the appropriate resolution class GoogleUserCircleAvatar extends StatelessWidget { @@ -33,7 +34,7 @@ class GoogleUserCircleAvatar extends StatelessWidget { /// /// The format is is "`/sNN-c/`", where `NN` is the max width/height of the /// image, and "`c`" indicates we want the image cropped. - static final RegExp sizeDirective = RegExp(r'^s[0-9]{1,5}(-c)?$'); + static final RegExp sizeDirective = fife.sizeDirective; /// The Google user's identity; guaranteed to be non-null. final GoogleIdentity identity; @@ -67,8 +68,7 @@ class GoogleUserCircleAvatar extends StatelessWidget { ); } - /// Adds sizing information to [photoUrl], inserted as the last path segment - /// before the image filename. The format is described in [sizeDirective]. + /// Adds correct sizing information to [photoUrl]. /// /// Falls back to the default profile photo if [photoUrl] is [null]. static String _sizedProfileImageUrl(String photoUrl, double size) { @@ -77,23 +77,7 @@ class GoogleUserCircleAvatar extends StatelessWidget { // the default profile photo as a last resort. return 'https://lh3.googleusercontent.com/a/default-user=s${size.round()}-c'; } - final Uri profileUri = Uri.parse(photoUrl); - final List pathSegments = - List.from(profileUri.pathSegments); - if (pathSegments.length <= 2) { - // New style URLs - pathSegments.last = '${pathSegments.last}=s${size.round()}-c'; - } else { - // Old style URLs - pathSegments - ..removeWhere(sizeDirective.hasMatch) - ..insert(pathSegments.length - 1, 's${size.round()}-c'); - } - return Uri( - scheme: profileUri.scheme, - host: profileUri.host, - pathSegments: pathSegments, - ).toString(); + return fife.addSizeDirectiveToUrl(photoUrl, size); } Widget _buildClippedImage(BuildContext context, BoxConstraints constraints) { diff --git a/packages/google_sign_in/lib/src/fife.dart b/packages/google_sign_in/lib/src/fife.dart new file mode 100644 index 000000000000..ab9756537dfa --- /dev/null +++ b/packages/google_sign_in/lib/src/fife.dart @@ -0,0 +1,53 @@ +/// A regular expression that matches against the "size directive" path +/// segment of Google profile image URLs. +/// +/// The format is is "`/sNN-c/`", where `NN` is the max width/height of the +/// image, and "`c`" indicates we want the image cropped. +final RegExp sizeDirective = RegExp(r'^s[0-9]{1,5}(-c)?$'); + +/// Adds sizing information to [photoUrl], inserted as the last path segment +/// before the image filename. The format is described in [sizeDirective]. +/// +/// Falls back to the default profile photo if [photoUrl] is [null]. +String addSizeDirectiveToUrl(String photoUrl, double size) { + if (photoUrl == null) { + // If the user has no profile photo and no display name, fall back to + // the default profile photo as a last resort. + return 'https://lh3.googleusercontent.com/a/default-user=s${size.round()}-c'; + } + final Uri profileUri = Uri.parse(photoUrl); + final List pathSegments = List.from(profileUri.pathSegments); + if (pathSegments.length <= 2) { + /// New URLs may have directives at the end of the URL, like "`=sNN-c`". + /// Each filter is separated by dashes. Filters may contain = signs: + /// "`=s120-c-fSoften=1,50,0`" + final String imagePath = pathSegments.last; + // Locate the first = + final int directiveSeparator = imagePath.indexOf('='); + if (directiveSeparator >= 0) { + // Split the image URL by the first = + final String image = imagePath.substring(0, directiveSeparator); + final String directive = imagePath.substring(directiveSeparator + 1); + // Split the second half by - + final Set directives = Set.from(directive.split('-')) + // Remove the size directive, if present, and empty values + ..removeWhere((String s) => s.isEmpty || sizeDirective.hasMatch(s)) + // Add the size and crop directives + ..addAll(['c', 's${size.round()}']); + + pathSegments.last = '$image=${directives.join("-")}'; + } else { + pathSegments.last = '${pathSegments.last}=c-s${size.round()}'; + } + } else { + // Old style URLs + pathSegments + ..removeWhere(sizeDirective.hasMatch) + ..insert(pathSegments.length - 1, 's${size.round()}-c'); + } + return Uri( + scheme: profileUri.scheme, + host: profileUri.host, + pathSegments: pathSegments, + ).toString(); +} diff --git a/packages/google_sign_in/test/fife.dart b/packages/google_sign_in/test/fife.dart new file mode 100644 index 000000000000..bfc4937a7c64 --- /dev/null +++ b/packages/google_sign_in/test/fife.dart @@ -0,0 +1,66 @@ +// Copyright 2019 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:flutter_test/flutter_test.dart'; +import 'package:google_sign_in/src/fife.dart'; + +void main() { + group('addSizeDirectiveToUrl', () { + const double size = 20; + + group('Old style URLs', () { + const String base = + 'https://lh3.googleusercontent.com/-ukEAtRyRhw8/AAAAAAAAAAI/AAAAAAAAAAA/ACHi3rfhID9XACtdb9q_xK43VSXQvBV11Q.CMID'; + const String expected = '$base/s20-c/photo.jpg'; + + test('with directives, sets size', () { + final String url = '$base/s64-c/photo.jpg'; + expect(addSizeDirectiveToUrl(url, size), expected); + }); + + test('no directives, sets size and crop', () { + final String url = '$base/photo.jpg'; + expect(addSizeDirectiveToUrl(url, size), expected); + }); + + test('no crop, sets size and crop', () { + final String url = '$base/s64/photo.jpg'; + expect(addSizeDirectiveToUrl(url, size), expected); + }); + }); + + group('New style URLs', () { + const String base = + 'https://lh3.googleusercontent.com/a-/AAuE7mC0Lh4F4uDtEaY7hpe-GIsbDpqfMZ3_2UhBQ8Qk'; + const String expected = '$base=c-s20'; + + test('with directives, sets size', () { + final String url = '$base=s120-c'; + expect(addSizeDirectiveToUrl(url, size), expected); + }); + + test('no directives, sets size and crop', () { + final String url = base; + expect(addSizeDirectiveToUrl(url, size), expected); + }); + + test('no directives, but with an equals sign, sets size and crop', () { + final String url = '$base='; + expect(addSizeDirectiveToUrl(url, size), expected); + }); + + test('no crop, adds crop', () { + final String url = '$base=s120'; + expect(addSizeDirectiveToUrl(url, size), expected); + }); + + test('many directives, sets size and crop, preserves other directives', + () { + final String url = '$base=s120-c-fSoften=1,50,0'; + final String expected = '$base=c-fSoften=1,50,0-s20'; + expect(addSizeDirectiveToUrl(url, size), expected); + }); + }); + }); +} From 6caa5e609dc4542c96c9a4610d63350c1edfaa21 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Fri, 8 Nov 2019 13:20:22 -0800 Subject: [PATCH 3/5] Fix documentation --- packages/google_sign_in/lib/src/fife.dart | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/google_sign_in/lib/src/fife.dart b/packages/google_sign_in/lib/src/fife.dart index ab9756537dfa..8b1121315901 100644 --- a/packages/google_sign_in/lib/src/fife.dart +++ b/packages/google_sign_in/lib/src/fife.dart @@ -5,16 +5,14 @@ /// image, and "`c`" indicates we want the image cropped. final RegExp sizeDirective = RegExp(r'^s[0-9]{1,5}(-c)?$'); -/// Adds sizing information to [photoUrl], inserted as the last path segment -/// before the image filename. The format is described in [sizeDirective]. +/// Adds [size] directive to [photoUrl]. /// -/// Falls back to the default profile photo if [photoUrl] is [null]. +/// Depending on the format of photoUrl, this might: +/// * Insert information as the last path segment (old style URLs), before +/// the image filename. The format is described in [sizeDirective]. +/// * Insert information at the end of the URL, after an = sign. The format +/// is described within this method. String addSizeDirectiveToUrl(String photoUrl, double size) { - if (photoUrl == null) { - // If the user has no profile photo and no display name, fall back to - // the default profile photo as a last resort. - return 'https://lh3.googleusercontent.com/a/default-user=s${size.round()}-c'; - } final Uri profileUri = Uri.parse(photoUrl); final List pathSegments = List.from(profileUri.pathSegments); if (pathSegments.length <= 2) { From c8028d303c6d7aa08e0ecfd1b2188cfd1a30358d Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Mon, 11 Nov 2019 16:00:03 -0800 Subject: [PATCH 4/5] Address PR feedback. --- .../google_sign_in/CHANGELOG.md | 2 +- packages/google_sign_in/lib/src/fife.dart | 51 +++++++++++++------ 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/packages/google_sign_in/google_sign_in/CHANGELOG.md b/packages/google_sign_in/google_sign_in/CHANGELOG.md index 77786459b7f5..1f7f5b605bb1 100644 --- a/packages/google_sign_in/google_sign_in/CHANGELOG.md +++ b/packages/google_sign_in/google_sign_in/CHANGELOG.md @@ -1,6 +1,6 @@ ## 4.0.13 -* Fix GoogleUserCircleAvatar to handle new style profile image URLs. +* Fix `GoogleUserCircleAvatar` to handle new style profile image URLs. ## 4.0.12 diff --git a/packages/google_sign_in/lib/src/fife.dart b/packages/google_sign_in/lib/src/fife.dart index 8b1121315901..14ecf5fd6083 100644 --- a/packages/google_sign_in/lib/src/fife.dart +++ b/packages/google_sign_in/lib/src/fife.dart @@ -1,3 +1,7 @@ +// Copyright 2019 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + /// A regular expression that matches against the "size directive" path /// segment of Google profile image URLs. /// @@ -5,35 +9,50 @@ /// image, and "`c`" indicates we want the image cropped. final RegExp sizeDirective = RegExp(r'^s[0-9]{1,5}(-c)?$'); -/// Adds [size] directive to [photoUrl]. +/// Adds [size] (and crop) directive to [photoUrl]. +/// +/// There are two formats for photoUrls coming from the Sign In backend. +/// +/// The two formats can be told apart by the number of path segments in the +/// URL (path segments: parts of the URL separated by slashes "/"): +/// +/// * If the URL has 2 or less path segments, it is a *new* style URL. +/// * If the URL has more than 2 path segments, it is an old style URL. +/// +/// Old style URLs encode the image transformation directives as the last +/// path segment. Look at the [sizeDirective] Regular Expression for more +/// information about these URLs. /// -/// Depending on the format of photoUrl, this might: -/// * Insert information as the last path segment (old style URLs), before -/// the image filename. The format is described in [sizeDirective]. -/// * Insert information at the end of the URL, after an = sign. The format -/// is described within this method. +/// New style URLs carry the same directives at the end of the URL, +/// after an = sign, like: "`=s120-c-fSoften=1,50,0`". +/// +/// Directives may contain the "=" sign (`fSoften=1,50,0`), but it seems the +/// base URL of the images don't. "Everything after the first = sign" is a +/// good heuristic to split new style URLs. +/// +/// Each directive is separated from others by dashes. Directives are the same +/// as described in the [sizeDirective] RegExp. +/// +/// Modified image URLs are recomposed by performing the parsing steps in reverse. String addSizeDirectiveToUrl(String photoUrl, double size) { final Uri profileUri = Uri.parse(photoUrl); final List pathSegments = List.from(profileUri.pathSegments); if (pathSegments.length <= 2) { - /// New URLs may have directives at the end of the URL, like "`=sNN-c`". - /// Each filter is separated by dashes. Filters may contain = signs: - /// "`=s120-c-fSoften=1,50,0`" final String imagePath = pathSegments.last; - // Locate the first = + // Does this have any existing transformation directives? final int directiveSeparator = imagePath.indexOf('='); if (directiveSeparator >= 0) { - // Split the image URL by the first = - final String image = imagePath.substring(0, directiveSeparator); + // Split the baseUrl from the sizing directive by the first "=" + final String baseUrl = imagePath.substring(0, directiveSeparator); final String directive = imagePath.substring(directiveSeparator + 1); - // Split the second half by - + // Split the directive by "-" final Set directives = Set.from(directive.split('-')) - // Remove the size directive, if present, and empty values + // Remove the size directive, if present, and any empty values ..removeWhere((String s) => s.isEmpty || sizeDirective.hasMatch(s)) // Add the size and crop directives ..addAll(['c', 's${size.round()}']); - - pathSegments.last = '$image=${directives.join("-")}'; + // Recompose the URL by performing the reverse of the parsing + pathSegments.last = '$baseUrl=${directives.join("-")}'; } else { pathSegments.last = '${pathSegments.last}=c-s${size.round()}'; } From 14c93267bc0a01a8c6fb13d9c2075d1a78c0997f Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Mon, 11 Nov 2019 16:12:23 -0800 Subject: [PATCH 5/5] Un-mess rebase Move lib and test files to their correct location. --- packages/google_sign_in/{ => google_sign_in}/lib/src/fife.dart | 0 .../{test/fife.dart => google_sign_in/test/fife_test.dart} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename packages/google_sign_in/{ => google_sign_in}/lib/src/fife.dart (100%) rename packages/google_sign_in/{test/fife.dart => google_sign_in/test/fife_test.dart} (100%) diff --git a/packages/google_sign_in/lib/src/fife.dart b/packages/google_sign_in/google_sign_in/lib/src/fife.dart similarity index 100% rename from packages/google_sign_in/lib/src/fife.dart rename to packages/google_sign_in/google_sign_in/lib/src/fife.dart diff --git a/packages/google_sign_in/test/fife.dart b/packages/google_sign_in/google_sign_in/test/fife_test.dart similarity index 100% rename from packages/google_sign_in/test/fife.dart rename to packages/google_sign_in/google_sign_in/test/fife_test.dart