Merge pull request #4053 from dpalou/MOBILE-3403

MOBILE-3403 core: Avoid performing requests to embedded untreated URLs
2024-05-17 13:26:53 +02:00 · 2024-05-17 13:26:53 +02:00 · e511629b9c
parent 7d862ea37d 6da34f5984
commit e511629b9c
18 changed files with 114 additions and 103 deletions
--- a/src/addons/badges/pages/issued-badge/issued-badge.html
+++ b/src/addons/badges/pages/issued-badge/issued-badge.html
@ -17,7 +17,7 @@
        <ion-item-group *ngIf="badge">
            <ion-item class="ion-text-wrap ion-text-center">
                <ion-label>
-                    <img *ngIf="badge.badgeurl" class="large-avatar" [src]="badge.badgeurl" core-external-content [alt]="badge.name" />
+                    <img *ngIf="badge.badgeurl" class="large-avatar" [url]="badge.badgeurl" core-external-content [alt]="badge.name" />
                    <ion-badge color="danger" *ngIf="badge.dateexpire && currentTime >= badge.dateexpire">
                        {{ 'addon.badges.expired' | translate }}
                    </ion-badge>
--- a/src/addons/badges/pages/user-badges/user-badges.html
+++ b/src/addons/badges/pages/user-badges/user-badges.html
@ -20,7 +20,7 @@
                <ion-item button class="ion-text-wrap" *ngFor="let badge of badges.items" [attr.aria-label]="badge.name"
                    (click)="badges.select(badge)" [attr.aria-current]="badges.getItemAriaCurrent(badge)" [detail]="true">
                    <ion-avatar slot="start">
-                        <img [src]="badge.badgeurl" [alt]="badge.name" core-external-content>
+                        <img [url]="badge.badgeurl" [alt]="badge.name" core-external-content>
                    </ion-avatar>
                    <ion-label>
                        <p class="item-heading">{{ badge.name }}</p>
--- a/src/addons/messages/components/conversation-info/conversation-info.html
+++ b/src/addons/messages/components/conversation-info/conversation-info.html
@ -19,7 +19,7 @@
        <ion-item class="ion-text-center" *ngIf="conversation">
            <ion-label>
                <div class="large-avatar">
-                    <img class="avatar" [src]="conversation.imageurl" core-external-content [alt]="conversation.name"
+                    <img class="avatar" [url]="conversation.imageurl" core-external-content [alt]="conversation.name"
                        onError="this.src='assets/img/group-avatar.svg'">
                </div>
                <h2>
--- a/src/addons/messages/pages/discussion/discussion.html
+++ b/src/addons/messages/pages/discussion/discussion.html
@ -5,7 +5,7 @@
        </ion-buttons>
        <ion-title>
            <h1>
-                <img *ngIf="loaded && !otherMember && conversationImage" class="core-bar-button-image" [src]="conversationImage" alt=""
+                <img *ngIf="loaded && !otherMember && conversationImage" class="core-bar-button-image" [url]="conversationImage" alt=""
                    onError="this.src='assets/img/group-avatar.svg'" core-external-content role="presentation" [siteId]="siteId">
                <core-user-avatar *ngIf="loaded && otherMember" class="core-bar-button-image" [user]="otherMember" [linkProfile]="false"
                    [checkOnline]="otherMember.showonlinestatus" />
--- a/src/addons/messages/pages/group-conversations/group-conversations.html
+++ b/src/addons/messages/pages/group-conversations/group-conversations.html
@ -85,7 +85,7 @@
        [attr.aria-label]="conversation.name">
        <!-- Group conversation image. -->
        <ion-avatar slot="start" *ngIf="conversation.type === typeGroup">
-            <img [src]="conversation.imageurl" [alt]="conversation.name" core-external-content
+            <img [url]="conversation.imageurl" [alt]="conversation.name" core-external-content
                onError="this.src='assets/img/group-avatar.svg'">
        </ion-avatar>

--- a/src/addons/mod/data/fields/picture/component/addon-mod-data-field-picture.html
+++ b/src/addons/mod/data/fields/picture/component/addon-mod-data-field-picture.html
@ -13,8 +13,8 @@
 </ng-container>

 <button class="as-link" *ngIf="listMode && imageUrl" (click)="navigateEntry()">
-    <img [src]="imageUrl" [alt]="title" class="core-media-adapt-width listMode_picture" core-external-content />
+    <img [url]="imageUrl" [alt]="title" class="core-media-adapt-width listMode_picture" core-external-content />
 </button>

-<img *ngIf="showMode && imageUrl" [src]="imageUrl" [alt]="title" class="core-media-adapt-width listMode_picture" [attr.width]="width"
+<img *ngIf="showMode && imageUrl" [url]="imageUrl" [alt]="title" class="core-media-adapt-width listMode_picture" [attr.width]="width"
    [attr.height]="height" core-external-content />
--- a/src/addons/notifications/pages/list/list.html
+++ b/src/addons/notifications/pages/list/list.html
@ -41,15 +41,15 @@
                    [profileUrl]="notification.profileimageurlfrom" [fullname]="notification.userfromfullname"
                    [userId]="notification.useridfrom">
                    <div class="core-avatar-extra-img" *ngIf="notification.iconurl">
-                        <img [src]="notification.iconurl" alt="" role="presentation" core-external-content>
+                        <img [url]="notification.iconurl" alt="" role="presentation" core-external-content>
                    </div>
                </core-user-avatar>

                <ng-container *ngIf="notification.useridfrom <= 0">
-                    <img *ngIf="notification.imgUrl" class="core-notification-img" [src]="notification.imgUrl" core-external-content alt=""
+                    <img *ngIf="notification.imgUrl" class="core-notification-img" [url]="notification.imgUrl" core-external-content alt=""
                        role="presentation" slot="start">
                    <div class="core-notification-icon" *ngIf="!notification.imgUrl" slot="start">
-                        <img *ngIf="notification.iconurl" [src]="notification.iconurl" core-external-content alt="" role="presentation">
+                        <img *ngIf="notification.iconurl" [url]="notification.iconurl" core-external-content alt="" role="presentation">
                        <ion-icon *ngIf="!notification.iconurl" name="fas-bell" aria-hidden="true" />
                    </div>
                </ng-container>
--- a/src/addons/notifications/pages/notification/notification.html
+++ b/src/addons/notifications/pages/notification/notification.html
@ -16,15 +16,15 @@
                <core-user-avatar *ngIf="notification.useridfrom > 0" slot="start" [userId]="notification.useridfrom"
                    [profileUrl]="notification.profileimageurlfrom" [fullname]="notification.userfromfullname">
                    <div class="core-avatar-extra-img" *ngIf="notification.iconurl">
-                        <img [src]="notification.iconurl" alt="" role="presentation" core-external-content>
+                        <img [url]="notification.iconurl" alt="" role="presentation" core-external-content>
                    </div>
                </core-user-avatar>

                <ng-container *ngIf="notification.useridfrom <= 0">
-                    <img *ngIf="notification.imgUrl" class="core-notification-img" [src]="notification.imgUrl" core-external-content alt=""
+                    <img *ngIf="notification.imgUrl" class="core-notification-img" [url]="notification.imgUrl" core-external-content alt=""
                        role="presentation" slot="start">
                    <div class="core-notification-icon" *ngIf="!notification.imgUrl" slot="start">
-                        <img *ngIf="notification.iconurl" [src]="notification.iconurl" core-external-content alt="" role="presentation">
+                        <img *ngIf="notification.iconurl" [url]="notification.iconurl" core-external-content alt="" role="presentation">
                        <ion-icon *ngIf="!notification.iconurl" name="fas-bell" aria-hidden="true" />
                    </div>
                </ng-container>
--- a/src/core/components/course-image/course-image.html
+++ b/src/core/components/course-image/course-image.html
@ -1,4 +1,4 @@
 <ion-icon *ngIf="!course.courseimage" name="fas-graduation-cap" slot="start" aria-hidden="true" />
 <ion-avatar *ngIf="course.courseimage" slot="start">
-    <img [src]="course.courseimage" core-external-content alt="" (error)="loadFallbackCourseIcon()" />
+    <img [url]="course.courseimage" core-external-content alt="" (error)="loadFallbackCourseIcon()" />
 </ion-avatar>
--- a/src/core/components/mod-icon/mod-icon.html
+++ b/src/core/components/mod-icon/mod-icon.html
@ -1,5 +1,5 @@
 <ng-container *ngIf="loaded && !svgLoaded">
-    <img *ngIf="!isLocalUrl" [src]="iconUrl" core-external-content alt="" [component]="linkIconWithComponent ? modname : null"
+    <img *ngIf="!isLocalUrl" [url]="iconUrl" core-external-content alt="" [component]="linkIconWithComponent ? modname : null"
        [componentId]="linkIconWithComponent ? componentId : null" (error)="loadFallbackIcon()">
    <img *ngIf="isLocalUrl" [src]="iconUrl" (error)="loadFallbackIcon()" alt="">
 </ng-container>
--- a/src/core/components/user-avatar/core-user-avatar.html
+++ b/src/core/components/user-avatar/core-user-avatar.html
@ -1,8 +1,8 @@
 <ng-container *ngIf="avatarUrl">
-    <img class="userpicture" *ngIf="linkProfile" [src]="avatarUrl" [alt]="'core.pictureof' | translate:{$a: fullname}" core-external-content
+    <img class="userpicture" *ngIf="linkProfile" [url]="avatarUrl" [alt]="'core.pictureof' | translate:{$a: fullname}" core-external-content
        (error)="loadImageError()" (ariaButtonClick)="gotoProfile($event)" [siteId]="siteId">

-    <img class="userpicture" *ngIf="!linkProfile" [src]="avatarUrl" [alt]="'core.pictureof' | translate:{$a: fullname}"
+    <img class="userpicture" *ngIf="!linkProfile" [url]="avatarUrl" [alt]="'core.pictureof' | translate:{$a: fullname}"
        core-external-content (error)="loadImageError()" aria-hidden="true" [siteId]="siteId">
 </ng-container>
 <ng-container *ngIf="!avatarUrl && initials">
--- a/src/core/directives/external-content.ts
+++ b/src/core/directives/external-content.ts
@ -60,9 +60,19 @@ export class CoreExternalContentDirective implements AfterViewInit, OnChanges, O
    @Input() siteId?: string; // Site ID to use.
    @Input() component?: string; // Component to link the file to.
    @Input() componentId?: string | number; // Component ID to use in conjunction with the component.
+    @Input() url?: string | null; // The URL to use in the element, either as src or href.
+    @Input() posterUrl?: string | null; // The poster URL.
+    /**
+     * @deprecated since 4.4. Use url instead.
+     */
    @Input() src?: string;
+    /**
+     * @deprecated since 4.4. Use url instead.
+     */
    @Input() href?: string;
-    @Input('target-src') targetSrc?: string; // eslint-disable-line @angular-eslint/no-input-rename
+    /**
+     * @deprecated since 4.4. Use posterUrl instead.
+     */
    @Input() poster?: string;
    @Output() onLoad = new EventEmitter(); // Emitted when content is loaded. Only for images.

@ -142,23 +152,22 @@ export class CoreExternalContentDirective implements AfterViewInit, OnChanges, O

        if (tagName === 'A' || tagName == 'IMAGE') {
            targetAttr = 'href';
-            url = this.href ?? '';
+            url = this.url ?? this.href ?? ''; // eslint-disable-line deprecation/deprecation

        } else if (tagName === 'IMG') {
            targetAttr = 'src';
-            url = this.src ?? '';
+            url = this.url ?? this.src ?? ''; // eslint-disable-line deprecation/deprecation

        } else if (tagName === 'AUDIO' || tagName === 'VIDEO' || tagName === 'SOURCE' || tagName === 'TRACK') {
            targetAttr = 'src';
-            url = (this.targetSrc || this.src) ?? '';
+            url = this.url ?? this.src ?? ''; // eslint-disable-line deprecation/deprecation

-            if (tagName === 'VIDEO') {
-                if (this.poster) {
-                    // Handle poster.
-                    this.handleExternalContent('poster', this.poster, siteId).catch(() => {
-                        // Ignore errors.
-                    });
-                }
+            if (tagName === 'VIDEO' && (this.posterUrl || this.poster)) { // eslint-disable-line deprecation/deprecation
+                // Handle poster.
+                // eslint-disable-next-line deprecation/deprecation
+                this.handleExternalContent('poster', this.posterUrl ?? this.poster ?? '', siteId).catch(() => {
+                    // Ignore errors.
+                });
            }

        } else {
@ -168,32 +177,11 @@ export class CoreExternalContentDirective implements AfterViewInit, OnChanges, O
            return;
        }

-        // Avoid handling data url's.
-        if (url && url.indexOf('data:') === 0) {
-            if (tagName === 'SOURCE') {
-                // Restoring original src.
-                this.addSource(url);
-            }
-
-            this.onLoad.emit();
-            this.loaded = true;
-            this.onReadyPromise.resolve();
-
-            return;
-        }
-
        try {
            await this.handleExternalContent(targetAttr, url, siteId);
        } catch (error) {
-            // Error handling content. Make sure the loaded event is triggered for images.
-            if (tagName === 'IMG') {
-                if (url) {
-                    this.waitForLoad();
-                } else {
-                    this.onLoad.emit();
-                    this.loaded = true;
-                }
-            }
+            // Error handling content. Make sure the original URL is set.
+           this.setElementUrl(targetAttr, url);
        } finally {
            this.onReadyPromise.resolve();
        }
@ -221,13 +209,6 @@ export class CoreExternalContentDirective implements AfterViewInit, OnChanges, O
                (tagName === 'A' && !(isSiteFile || site.isSiteThemeImageUrl(url) || CoreUrlUtils.isGravatarUrl(url)))) {

            this.logger.debug('Ignoring non-downloadable URL: ' + url);
-            if (tagName === 'SOURCE') {
-                // Restoring original src.
-                this.addSource(url);
-            } else if (url && !this.element.getAttribute(targetAttr)) {
-                // By default, Angular inputs aren't added as DOM attributes. Add it now.
-                this.element.setAttribute(targetAttr, url);
-            }

            throw new CoreError('Non-downloadable URL');
        }
@ -241,28 +222,56 @@ export class CoreExternalContentDirective implements AfterViewInit, OnChanges, O
        const finalUrl = await this.getUrlToUse(targetAttr, url, site);

        this.logger.debug('Using URL ' + finalUrl + ' for ' + url);
-        if (tagName === 'SOURCE') {
-            // The browser does not catch changes in SRC, we need to add a new source.
-            this.addSource(finalUrl);
-        } else {
-            if (tagName === 'IMG') {
-                this.loaded = false;
-                this.waitForLoad();
-            }

-            if (targetAttr == 'poster') {
-                // Setting the poster immediately doesn't display it in some cases. Set it to empty and then set the right one.
-                this.element.setAttribute(targetAttr, '');
-                await CoreUtils.nextTick();
-            }
-
-            this.element.setAttribute(targetAttr, finalUrl);
-            this.element.setAttribute('data-original-' + targetAttr, url);
-        }
+        this.setElementUrl(targetAttr, finalUrl);

        this.setListeners(targetAttr, url, site);
    }

+    /**
+     * Set the URL to the element.
+     *
+     * @param targetAttr Name of the attribute to set.
+     * @param url URL to set.
+     */
+    protected setElementUrl(targetAttr: string, url: string): void {
+        if (!url) {
+            // Ignore empty URLs.
+            if (this.element.tagName === 'IMG') {
+                this.onLoad.emit();
+                this.loaded = true;
+            }
+
+            return;
+        }
+
+        if (this.element.tagName === 'SOURCE') {
+            // The WebView does not detect changes in SRC, we need to add a new source.
+            this.addSource(url);
+        } else {
+            this.element.setAttribute(targetAttr, url);
+
+            const originalUrl = targetAttr === 'poster' ?
+                (this.posterUrl ?? this.poster) : // eslint-disable-line deprecation/deprecation
+                (this.url ?? this.src ?? this.href); // eslint-disable-line deprecation/deprecation
+            if (originalUrl && originalUrl !== url) {
+                this.element.setAttribute('data-original-' + targetAttr, originalUrl);
+            }
+        }
+
+        if (this.element.tagName !== 'IMG') {
+            return;
+        }
+
+        if (url.startsWith('data:')) {
+            this.onLoad.emit();
+            this.loaded = true;
+        } else {
+            this.loaded = false;
+            this.waitForLoad();
+        }
+    }
+
    /**
     * Handle inline styles, trying to download referenced files.
     *
--- a/src/core/directives/format-text.ts
+++ b/src/core/directives/format-text.ts
@ -180,10 +180,14 @@ export class CoreFormatTextDirective implements OnChanges, OnDestroy, AsyncDirec
        extContent.component = this.component;
        extContent.componentId = this.componentId;
        extContent.siteId = this.siteId;
-        extContent.src = element.getAttribute('src') || undefined;
-        extContent.href = element.getAttribute('href') || element.getAttribute('xlink:href') || undefined;
-        extContent.targetSrc = element.getAttribute('target-src') || undefined;
-        extContent.poster = element.getAttribute('poster') || undefined;
+        extContent.url = element.getAttribute('src') ?? element.getAttribute('href') ?? element.getAttribute('xlink:href');
+        extContent.posterUrl = element.getAttribute('poster');
+
+        // Remove the original attributes to avoid performing requests to untreated URLs.
+        element.removeAttribute('src');
+        element.removeAttribute('href');
+        element.removeAttribute('xlink:href');
+        element.removeAttribute('poster');

        extContent.ngAfterViewInit();

@ -721,6 +725,10 @@ export class CoreFormatTextDirective implements OnChanges, OnDestroy, AsyncDirec
     * @param isVideo Whether it's a video.
     */
    protected treatMedia(element: HTMLElement, isVideo: boolean = false): void {
+        if (isVideo) {
+            this.fixVideoSrcPlaceholder(element);
+        }
+
        this.addMediaAdaptClass(element);
        this.addExternalContent(element);

@ -738,18 +746,8 @@ export class CoreFormatTextDirective implements OnChanges, OnDestroy, AsyncDirec

        const sources = Array.from(element.querySelectorAll('source'));
        const tracks = Array.from(element.querySelectorAll('track'));
-        const hasPoster = isVideo && !!element.getAttribute('poster');
-
-        if (isVideo && !hasPoster) {
-            this.fixVideoSrcPlaceholder(element);
-        }

        sources.forEach((source) => {
-            if (isVideo && !hasPoster) {
-                this.fixVideoSrcPlaceholder(source);
-            }
-            source.setAttribute('target-src', source.getAttribute('src') || '');
-            source.removeAttribute('src');
            this.addExternalContent(source);
        });

@ -766,19 +764,23 @@ export class CoreFormatTextDirective implements OnChanges, OnDestroy, AsyncDirec
    /**
     * Try to fix the placeholder displayed when a video doesn't have a poster.
     *
-     * @param element Element to fix.
+     * @param videoElement Element to fix.
     */
-    protected fixVideoSrcPlaceholder(element: HTMLElement): void {
-        const src = element.getAttribute('src');
-        if (!src) {
+    protected fixVideoSrcPlaceholder(videoElement: HTMLElement): void {
+        if (videoElement.getAttribute('poster')) {
+            // Video has a poster, nothing to fix.
            return;
        }

-        if (src.match(/#t=\d/)) {
-            return;
-        }
+        // Fix the video and its sources.
+        [videoElement].concat(Array.from(videoElement.querySelectorAll('source'))).forEach((element) => {
+            const src = element.getAttribute('src');
+            if (!src || src.match(/#t=\d/)) {
+                return;
+            }

-        element.setAttribute('src', src + '#t=0.001');
+            element.setAttribute('src', src + '#t=0.001');
+        });
    }

    /**
--- a/src/core/features/course/pages/course-summary/course-summary.html
+++ b/src/core/features/course/pages/course-summary/course-summary.html
@ -17,7 +17,7 @@
    </ion-refresher>
    <core-loading [hideUntil]="dataLoaded">
        <div *ngIf="course" class="core-course-thumb" #courseThumb>
-            <img *ngIf="course.courseimage" [src]="course.courseimage" core-external-content alt="" (error)="loadFallbackCourseIcon()" />
+            <img *ngIf="course.courseimage" [url]="course.courseimage" core-external-content alt="" (error)="loadFallbackCourseIcon()" />
            <ion-icon *ngIf="!course.courseimage" name="fas-graduation-cap" class="course-icon" aria-hidden="true" />
        </div>
        <div *ngIf="course" class="course-container">
--- a/src/core/features/course/pages/index/index.html
+++ b/src/core/features/course/pages/index/index.html
@ -25,7 +25,7 @@
                <ion-icon name="fas-graduation-cap" class="course-icon" aria-hidden="true" />
            </div>
            <ion-avatar *ngIf="course.courseimage" slot="start" class="core-course-thumb">
-                <img [src]="course.courseimage" core-external-content alt="" />
+                <img [url]="course.courseimage" core-external-content alt="" />
            </ion-avatar>
        </ng-container>

--- a/src/core/features/courses/components/course-list-item/core-courses-course-list-item.html
+++ b/src/core/features/courses/components/course-list-item/core-courses-course-list-item.html
@ -3,7 +3,7 @@
    button [attr.aria-label]="course.displayname || course.fullname">

    <div *ngIf="layout === 'card' || layout === 'summarycard'" class="core-course-thumb" [class.core-course-color-img]="course.courseimage">
-        <img *ngIf="course.courseimage" [src]="course.courseimage" core-external-content alt="" (error)="loadFallbackCourseIcon()" />
+        <img *ngIf="course.courseimage" [url]="course.courseimage" core-external-content alt="" (error)="loadFallbackCourseIcon()" />
        <ion-icon *ngIf="!course.courseimage" name="fas-graduation-cap" class="course-icon" aria-hidden="true" />
    </div>

@ -32,7 +32,7 @@
            <ion-icon *ngIf="!course.courseimage" name="fas-graduation-cap" slot="start" class="course-icon core-course-thumb"
                aria-hidden="true" />
            <ion-avatar *ngIf="course.courseimage" slot="start" class="core-course-thumb">
-                <img [src]="course.courseimage" core-external-content alt="" (error)="loadFallbackCourseIcon()" />
+                <img [url]="course.courseimage" core-external-content alt="" (error)="loadFallbackCourseIcon()" />
            </ion-avatar>
        </ng-container>

--- a/src/core/features/search/components/global-search-result/global-search-result.html
+++ b/src/core/features/search/components/global-search-result/global-search-result.html
@ -6,7 +6,7 @@
            <ion-icon *ngIf="renderedIcon" [name]="renderedIcon" aria-hidden="true" />
            <core-mod-icon *ngIf="!renderedIcon && result.module" [modicon]="result.module.iconurl" [modname]="result.module.name"
                [colorize]="false" />
-            <img *ngIf="!renderedIcon && !result.module && result.component" [src]="result.component.iconurl" alt="" class="result-icon"
+            <img *ngIf="!renderedIcon && !result.module && result.component" [url]="result.component.iconurl" alt="" class="result-icon"
                core-external-content [component]="result.component.name">
            <core-format-text [text]="result.title" />
        </h3>
--- a/src/core/features/viewer/components/image/image.html
+++ b/src/core/features/viewer/components/image/image.html
@ -2,7 +2,7 @@
    <swiper-container #swiperRef>
        <swiper-slide>
            <div class="swiper-zoom-container">
-                <img [src]="image" [alt]="title" core-external-content [component]="component" [componentId]="componentId">
+                <img [url]="image" [alt]="title" core-external-content [component]="component" [componentId]="componentId">
            </div>
        </swiper-slide>
    </swiper-container>