Merge pull request #4615 from nextcloud/feat/base64-routing

st3iny · web-flow · commit 96c251a36aa1 · 2025-08-22T23:21:07.000+02:00
fix: routing based on base64 encoding
diff --git a/lib/ContactsMenu/Providers/DetailsProvider.php b/lib/ContactsMenu/Providers/DetailsProvider.php
@@ -82,7 +82,7 @@ public function process(IEntry $entry) {
 
 		$contactsUrl = $this->urlGenerator->getAbsoluteURL(
 			$this->urlGenerator->linkToRoute('contacts.contacts.direct', [
-				'contact' => $uid . '~' . $addressBookUri
+				'contact' => base64_encode($uid . '~' . $addressBookUri),
 			])
 		);
 
diff --git a/lib/Controller/ContactsController.php b/lib/Controller/ContactsController.php
@@ -33,6 +33,11 @@ public function __construct(
 	 * @param string $uuid
 	 */
 	public function direct(string $contact): RedirectResponse {
+		// Keep compatibility with old routing scheme
+		if (str_contains($contact, '~')) {
+			$contact = base64_encode($contact);
+		}
+
 		$url = $this->urlGenerator->getAbsoluteURL(
 			$this->urlGenerator->linkToRoute('contacts.page.index') . $this->l10n->t('All contacts') . '/' . $contact
 		);
diff --git a/src/components/AppContent/ChartContent.vue b/src/components/AppContent/ChartContent.vue
@@ -36,7 +36,7 @@ export default {
 		transformData() {
 			const contactsByUid = {}
 			const contacts = Object.keys(this.contactsList).map(key => {
-				const [uid, addressbook] = key.split('~')
+				const [uid, addressbook] = Buffer.from(key, 'base64').toString('utf-8').split('~')
 				if (!contactsByUid[addressbook]) {
 					contactsByUid[addressbook] = {}
 				}
diff --git a/src/components/AppContent/ContactsContent.vue b/src/components/AppContent/ContactsContent.vue
@@ -116,10 +116,6 @@ export default {
 			return this.$store.getters.getSortedContacts
 		},
 
-		selectedContact() {
-			return this.$route.params.selectedContact
-		},
-
 		/**
 		 * Is this a real group ?
 		 * Aka not a dynamically generated one like `All contacts`
diff --git a/src/components/AppNavigation/GroupNavigationItem.vue b/src/components/AppNavigation/GroupNavigationItem.vue
@@ -151,9 +151,9 @@ export default {
 		async onDrop(event, group) {
 			try {
 				const contactFromDropData = JSON.parse(event.dataTransfer.getData('item'))
-				const contactFromStore = this.$store.getters.getContact(`${contactFromDropData.uid}~${contactFromDropData.addressbookId}`)
+				const contactFromStore = this.$store.getters.getContact(Buffer.from(`${contactFromDropData.uid}~${contactFromDropData.addressbookId}`, 'utf-8').toString('base64'))
 				if (contactFromStore && !this.isInGroup(contactFromStore.groups, group.id)) {
-					const contact = this.$store.getters.getContact(`${contactFromDropData.uid}~${contactFromDropData.addressbookId}`)
+					const contact = this.$store.getters.getContact(Buffer.from(`${contactFromDropData.uid}~${contactFromDropData.addressbookId}`, 'utf-8').toString('base64'))
 					await this.$store.dispatch('updateContactGroups', {
 						groupNames: [...contactFromStore.groups, group.id],
 						contact,
diff --git a/src/components/ContactDetails/ContactDetailsProperty.vue b/src/components/ContactDetails/ContactDetailsProperty.vue
@@ -296,7 +296,7 @@ export default {
 				}
 				if (this.propName === 'x-managersname') {
 					if (this.property.getParameter('uid')) {
-						return this.property.getParameter('uid') + '~' + this.contact.addressbook.id
+						return Buffer.from(this.property.getParameter('uid') + '~' + this.contact.addressbook.id, 'utf-8').toString('base64')
 					}
 					// Try to find the matching contact by display name
 					// TODO: this only *shows* the display name but doesn't assign the missing UID
diff --git a/src/components/ContactsList.vue b/src/components/ContactsList.vue
@@ -89,10 +89,15 @@ import Merging from './ContactsList/Merging.vue'
 import IconCancelRaw from '@mdi/svg/svg/cancel.svg?raw'
 // eslint-disable-next-line import/no-unresolved
 import IconDeleteRaw from '@mdi/svg/svg/delete-outline.svg'
+import RouterMixin from '../mixins/RouterMixin.js'
 
 export default {
 	name: 'ContactsList',
 
+	mixins: [
+		RouterMixin,
+	],
+
 	components: {
 		AppContentList,
 		NcNoteCard,
@@ -153,12 +158,6 @@ export default {
 	},
 
 	computed: {
-		selectedContact() {
-			return this.$route.params.selectedContact
-		},
-		selectedGroup() {
-			return this.$route.params.selectedGroup
-		},
 		filteredList() {
 			const contactsList = this.list
 				.filter(item => this.matchSearch(this.contacts[item.key]))
@@ -242,7 +241,7 @@ export default {
 		 * @param {string} key the contact unique key
 		 */
 		scrollToContact(key) {
-			const item = this.$el.querySelector('#' + btoa(key).slice(0, -2))
+			const item = this.$el.querySelector('#' + key.slice(0, -2))
 
 			// if the item is not visible in the list or barely visible
 			if (!(item && item.getBoundingClientRect().y > 50)) { // header height
diff --git a/src/components/ContactsList/ContactsListItem.vue b/src/components/ContactsList/ContactsListItem.vue
@@ -49,10 +49,15 @@ import {
 	NcAvatar,
 } from '@nextcloud/vue'
 import CheckIcon from 'vue-material-design-icons/Check.vue'
+import RouterMixin from '../../mixins/RouterMixin.js'
 
 export default {
 	name: 'ContactsListItem',
 
+	mixins: [
+		RouterMixin,
+	],
+
 	components: {
 		ListItem,
 		NcAvatar,
@@ -86,19 +91,13 @@ export default {
 	},
 
 	computed: {
-		selectedGroup() {
-			return this.$route.params.selectedGroup
-		},
-		selectedContact() {
-			return this.$route.params.selectedContact
-		},
 		// contact is not draggable when it has not been saved on server as it can't be added to groups/circles before
 		isDraggable() {
 			return !!this.source.dav && this.source.addressbook.id !== 'z-server-generated--system'
 		},
 		// usable and valid html id for scrollTo
 		id() {
-			return window.btoa(this.source.key).slice(0, -2)
+			return this.source.key.slice(0, -2)
 		},
 		getTel() {
 			return this.source.properties.find(property => property.name === 'tel')?.getFirstValue()
diff --git a/src/mixins/RouterMixin.js b/src/mixins/RouterMixin.js
@@ -14,5 +14,8 @@ export default {
 		selectedCircle() {
 			return this.$route.params.selectedCircle
 		},
+		selectedChart() {
+			return this.$route.params.selectedChart
+		},
 	},
 }
diff --git a/src/models/contact.js b/src/models/contact.js
@@ -199,7 +199,7 @@ export default class Contact {
 	 * @memberof Contact
 	 */
 	get key() {
-		return this.uid + '~' + this.addressbook.id
+		return Buffer.from(this.uid + '~' + this.addressbook.id, 'utf8').toString('base64')
 	}
 
 	/**
diff --git a/src/views/Contacts.vue b/src/views/Contacts.vue
@@ -70,6 +70,8 @@ import RootNavigation from '../components/AppNavigation/RootNavigation.vue'
 import SettingsImportContacts from '../components/AppNavigation/Settings/SettingsImportContacts.vue'
 import IconAdd from 'vue-material-design-icons/Plus.vue'
 
+import RouterMixin from '../mixins/RouterMixin.js'
+
 import Contact from '../models/contact.js'
 import rfcProps from '../models/rfcProps.js'
 
@@ -99,28 +101,9 @@ export default {
 
 	mixins: [
 		IsMobileMixin,
+		RouterMixin,
 	],
 
-	// passed by the router
-	props: {
-		selectedCircle: {
-			type: String,
-			default: undefined,
-		},
-		selectedGroup: {
-			type: String,
-			default: undefined,
-		},
-		selectedContact: {
-			type: String,
-			default: undefined,
-		},
-		selectedChart: {
-			type: String,
-			default: undefined,
-		},
-	},
-
 	data() {
 		return {
 			// The object shorthand syntax is breaking builds (bug in @babel/preset-env)
@@ -376,7 +359,7 @@ export default {
 			}
 
 			const inList = this.contactsList.findIndex(contact => contact.key === this.selectedContact) > -1
-			if (this.selectedContact === undefined || !inList) {
+			if (!this.selectedContact || !inList) {
 				// Unknown contact
 				if (this.selectedContact && !inList) {
 					showError(t('contacts', 'Contact not found'))
diff --git a/tests/unit/ContactsMenu/Provider/DetailsProviderTest.php b/tests/unit/ContactsMenu/Provider/DetailsProviderTest.php
@@ -63,7 +63,7 @@ public function testProcessContact() {
 		$uid = 'e3a71614-c602-4eb5-9994-47eec551542b';
 		$abUri = 'contacts-1';
 		$iconUrl = 'core/img/actions/info.svg';
-		$resultUri = "$domain/index.php/apps/contacts/direct/contact/$uid~$abUri";
+		$resultUri = "$domain/index.php/apps/contacts/direct/contact/ZTNhNzE2MTQtYzYwMi00ZWI1LTk5OTQtNDdlZWM1NTE1NDJifmNvbnRhY3RzLTE=";
 
 		$entry->expects($this->exactly(3))
 			->method('getProperty')
@@ -95,16 +95,93 @@ public function testProcessContact() {
 		$this->urlGenerator->expects($this->once())
 			->method('linkToRoute')
 			->with('contacts.contacts.direct', [
-				'contact' => $uid . '~' . $abUri
+				'contact' => 'ZTNhNzE2MTQtYzYwMi00ZWI1LTk5OTQtNDdlZWM1NTE1NDJifmNvbnRhY3RzLTE=',
 			])
-			->willReturn("/apps/contacts/direct/contact/$uid~$abUri");
+			->willReturn('/apps/contacts/direct/contact/ZTNhNzE2MTQtYzYwMi00ZWI1LTk5OTQtNDdlZWM1NTE1NDJifmNvbnRhY3RzLTE=');
 
 		// Action icon and contact absolute urls
 		$this->urlGenerator->expects($this->exactly(2))
 			->method('getAbsoluteURL')
 			->will($this->returnValueMap([
 				[$iconUrl, "$domain/$iconUrl"],
-				["/apps/contacts/direct/contact/$uid~$abUri", $resultUri]
+				['/apps/contacts/direct/contact/ZTNhNzE2MTQtYzYwMi00ZWI1LTk5OTQtNDdlZWM1NTE1NDJifmNvbnRhY3RzLTE=', $resultUri]
+			]));
+
+		// Translations
+		$this->l10n->expects($this->once())
+			->method('t')
+			->with('Details')
+			->willReturnArgument(0);
+
+		$this->actionFactory->expects($this->once())
+			->method('newLinkAction')
+			->with($this->equalTo("$domain/$iconUrl"), $this->equalTo('Details'), $this->equalTo($resultUri))
+			->willReturn($action);
+		$action->expects($this->once())
+			->method('setPriority')
+			->with($this->equalTo(0));
+		$entry->expects($this->once())
+			->method('addAction')
+			->with($action);
+
+		$this->provider->process($entry);
+	}
+
+	public function testProcessContactWithUnicodeUid() {
+		$entry = $this->createMock(IEntry::class);
+		$action = $this->createMock(ILinkAction::class);
+		$addressbook = $this->createMock(IAddressBook::class);
+
+		// DATA
+		$domain = 'https://cloud.example.com';
+		$uid = 'e3a71614-c602-4eb5-9994-47eec551542b-é';
+		$abUri = 'contacts-1';
+		$iconUrl = 'core/img/actions/info.svg';
+		$resultUri = "$domain/index.php/apps/contacts/direct/contact/ZTNhNzE2MTQtYzYwMi00ZWI1LTk5OTQtNDdlZWM1NTE1NDJiLcOpfmNvbnRhY3RzLTE=";
+
+
+		$entry->expects($this->exactly(3))
+			->method('getProperty')
+			->will($this->returnValueMap([
+				['UID', $uid],
+				['isLocalSystemBook', null],
+				['addressbook-key', 1]
+			]));
+
+		$addressbook->expects($this->once())
+			->method('getKey')
+			->willReturn(1);
+
+		$addressbook->expects($this->once())
+			->method('getUri')
+			->willReturn($abUri);
+
+		$this->manager->expects($this->once())
+			->method('getUserAddressbooks')
+			->willReturn([1 => $addressbook]);
+
+		// Action icon
+		$this->urlGenerator->expects($this->once())
+			->method('imagePath')
+			->with('core', 'actions/info.svg')
+			->willReturn($iconUrl);
+
+		//
+		$this->urlGenerator->expects($this->once())
+			->method('linkToRoute')
+			->with('contacts.contacts.direct', [
+				// Taken from node: Buffer.from('e3a71614-c602-4eb5-9994-47eec551542b-é~contacts-1', 'utf-8').toString('base64')
+				// To ensure interop between JavaScript and PHP
+				'contact' => 'ZTNhNzE2MTQtYzYwMi00ZWI1LTk5OTQtNDdlZWM1NTE1NDJiLcOpfmNvbnRhY3RzLTE=',
+			])
+			->willReturn('/apps/contacts/direct/contact/ZTNhNzE2MTQtYzYwMi00ZWI1LTk5OTQtNDdlZWM1NTE1NDJiLcOpfmNvbnRhY3RzLTE=');
+
+		// Action icon and contact absolute urls
+		$this->urlGenerator->expects($this->exactly(2))
+			->method('getAbsoluteURL')
+			->will($this->returnValueMap([
+				[$iconUrl, "$domain/$iconUrl"],
+				['/apps/contacts/direct/contact/ZTNhNzE2MTQtYzYwMi00ZWI1LTk5OTQtNDdlZWM1NTE1NDJiLcOpfmNvbnRhY3RzLTE=', $resultUri]
 			]));
 
 		// Translations
diff --git a/tests/unit/Controller/ContactsControllerTest.php b/tests/unit/Controller/ContactsControllerTest.php
@@ -12,6 +12,7 @@
 use OCP\IL10N;
 use OCP\IRequest;
 use OCP\IURLGenerator;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\MockObject\MockObject;
 
 class ContactsControllerTest extends TestCase {
@@ -38,10 +39,17 @@ protected function setUp(): void {
 		);
 	}
 
+	public static function provideDirectContactData(): array {
+		return [
+			['uuid~addressbook', 'dXVpZH5hZGRyZXNzYm9vaw=='],
+			['dXVpZH5hZGRyZXNzYm9vaw==', 'dXVpZH5hZGRyZXNzYm9vaw=='],
+		];
+	}
 
-	public function testRedirect() {
-		$contact = 'uuid~addressbook';
-
+	/**
+	 * @dataProvider provideDirectContactData
+	 */
+	public function testRedirect(string $contact, string $expectedContact): void {
 		$this->l10n->method('t')
 			->with('All contacts')
 			->willReturn('All contacts');
@@ -53,11 +61,11 @@ public function testRedirect() {
 
 		$this->urlGenerator->expects($this->once())
 			->method('getAbsoluteURL')
-			->with('/index.php/apps/contacts/All contacts/' . $contact)
-			->willReturn('/index.php/apps/contacts/All contacts/' . $contact);
+			->with('/index.php/apps/contacts/All contacts/' . $expectedContact)
+			->willReturn('/index.php/apps/contacts/All contacts/' . $expectedContact);
 
-		$result = $this->controller->direct('uuid~addressbook');
+		$result = $this->controller->direct($contact);
 		$this->assertTrue($result instanceof RedirectResponse);
-		$this->assertEquals('/index.php/apps/contacts/All contacts/' . $contact, $result->getRedirectURL());
+		$this->assertEquals('/index.php/apps/contacts/All contacts/' . $expectedContact, $result->getRedirectURL());
 	}
 }

Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@ export default {`
`36`	`36`	`transformData() {`
`37`	`37`	`const contactsByUid = {}`
`38`	`38`	`const contacts = Object.keys(this.contactsList).map(key => {`
`39`		`- const [uid, addressbook] = key.split('~')`
	`39`	`+ const [uid, addressbook] = Buffer.from(key, 'base64').toString('utf-8').split('~')`
`40`	`40`	`if (!contactsByUid[addressbook]) {`
`41`	`41`	`contactsByUid[addressbook] = {}`
`42`	`42`	`}`
Original file line number	Diff line number	Diff line change
`@@ -296,7 +296,7 @@ export default {`
`296`	`296`	`}`
`297`	`297`	`if (this.propName === 'x-managersname') {`
`298`	`298`	`if (this.property.getParameter('uid')) {`
`299`		`- return this.property.getParameter('uid') + '~' + this.contact.addressbook.id`
	`299`	`+ return Buffer.from(this.property.getParameter('uid') + '~' + this.contact.addressbook.id, 'utf-8').toString('base64')`
`300`	`300`	`}`
`301`	`301`	`// Try to find the matching contact by display name`
`302`	`302`	`// TODO: this only shows the display name but doesn't assign the missing UID`
Original file line number	Diff line number	Diff line change
`@@ -14,5 +14,8 @@ export default {`
`14`	`14`	`selectedCircle() {`
`15`	`15`	`return this.$route.params.selectedCircle`
`16`	`16`	`},`
	`17`	`+ selectedChart() {`
	`18`	`+ return this.$route.params.selectedChart`
	`19`	`+ },`
`17`	`20`	`},`
`18`	`21`	`}`
Original file line number	Diff line number	Diff line change
`@@ -199,7 +199,7 @@ export default class Contact {`
`199`	`199`	`* @memberof Contact`
`200`	`200`	`*/`
`201`	`201`	`get key() {`
`202`		`- return this.uid + '~' + this.addressbook.id`
	`202`	`+ return Buffer.from(this.uid + '~' + this.addressbook.id, 'utf8').toString('base64')`
`203`	`203`	`}`
`204`	`204`
`205`	`205`	`/**`