From c3326006336cbc56335d3e8f5fde9af49b5b2280 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Fri, 24 May 2024 14:21:57 +0200 Subject: [PATCH] 114624: Made the legacyBitstreamURLRedirectGuard return false for valid bitstream urls in combination with a HardRedirectService#redirect, this will make ensure the redirect is visible for curl instead of being performed by Angular (cherry picked from commit 23644e9ec70bef8bfbb3159412c56314e89c2106) --- ...egacy-bitstream-url-redirect.guard.spec.ts | 53 ++++++++++--------- .../legacy-bitstream-url-redirect.guard.ts | 33 ++++++------ src/app/core/data/bitstream-data.service.ts | 2 +- 3 files changed, 48 insertions(+), 40 deletions(-) diff --git a/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.spec.ts b/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.spec.ts index 8ca14231e8..38be4dbb38 100644 --- a/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.spec.ts +++ b/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.spec.ts @@ -1,14 +1,15 @@ +import { cold } from 'jasmine-marbles'; import { EMPTY } from 'rxjs'; + +import { PAGE_NOT_FOUND_PATH } from '../app-routing-paths'; import { BitstreamDataService } from '../core/data/bitstream-data.service'; import { RemoteData } from '../core/data/remote-data'; import { RequestEntryState } from '../core/data/request-entry-state.model'; -import { legacyBitstreamURLRedirectGuard } from './legacy-bitstream-url-redirect.guard'; -import { RouterStub } from '../shared/testing/router.stub'; -import { ServerResponseServiceStub } from '../shared/testing/server-response-service.stub'; -import { fakeAsync } from '@angular/core/testing'; -import { cold } from 'jasmine-marbles'; -import { PAGE_NOT_FOUND_PATH } from '../app-routing-paths'; +import { BrowserHardRedirectService } from '../core/services/browser-hard-redirect.service'; +import { HardRedirectService } from '../core/services/hard-redirect.service'; import { Bitstream } from '../core/shared/bitstream.model'; +import { RouterStub } from '../shared/testing/router.stub'; +import { legacyBitstreamURLRedirectGuard } from './legacy-bitstream-url-redirect.guard'; describe('legacyBitstreamURLRedirectGuard', () => { let resolver: any; @@ -16,21 +17,26 @@ describe('legacyBitstreamURLRedirectGuard', () => { let remoteDataMocks: { [type: string]: RemoteData }; let route; let state; - let serverResponseService: ServerResponseServiceStub; + let hardRedirectService: HardRedirectService; let router: RouterStub; + let bitstream: Bitstream; + beforeEach(() => { route = { params: {}, queryParams: {} }; router = new RouterStub(); - serverResponseService = new ServerResponseServiceStub(); + hardRedirectService = new BrowserHardRedirectService(window.location); state = {}; + bitstream = Object.assign(new Bitstream(), { + uuid: 'bitstream-id', + }); remoteDataMocks = { RequestPending: new RemoteData(undefined, 0, 0, RequestEntryState.RequestPending, undefined, undefined, undefined), ResponsePending: new RemoteData(undefined, 0, 0, RequestEntryState.ResponsePending, undefined, undefined, undefined), - Success: new RemoteData(0, 0, 0, RequestEntryState.Success, undefined, new Bitstream(), 200), + Success: new RemoteData(0, 0, 0, RequestEntryState.Success, undefined, bitstream, 200), NoContent: new RemoteData(0, 0, 0, RequestEntryState.Success, undefined, undefined, 204), Error: new RemoteData(0, 0, 0, RequestEntryState.Error, 'Internal server error', undefined, 500), }; @@ -54,7 +60,7 @@ describe('legacyBitstreamURLRedirectGuard', () => { }); }); it(`should call findByItemHandle with the handle, sequence id, and filename from the route`, () => { - resolver(route, state, bitstreamDataService, serverResponseService, router); + resolver(route, state, bitstreamDataService, hardRedirectService, router); expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( `${route.params.prefix}/${route.params.suffix}`, route.params.sequence_id, @@ -79,7 +85,7 @@ describe('legacyBitstreamURLRedirectGuard', () => { }); }); it(`should call findByItemHandle with the handle and filename from the route, and the sequence ID from the queryParams`, () => { - resolver(route, state, bitstreamDataService, serverResponseService, router); + resolver(route, state, bitstreamDataService, hardRedirectService, router); expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( `${route.params.prefix}/${route.params.suffix}`, route.queryParams.sequenceId, @@ -99,7 +105,7 @@ describe('legacyBitstreamURLRedirectGuard', () => { }); }); it(`should call findByItemHandle with the handle, and filename from the route`, () => { - resolver(route, state, bitstreamDataService, serverResponseService, router); + resolver(route, state, bitstreamDataService, hardRedirectService, router); expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( `${route.params.prefix}/${route.params.suffix}`, undefined, @@ -109,45 +115,44 @@ describe('legacyBitstreamURLRedirectGuard', () => { }); }); describe('should return and complete after the RemoteData has...', () => { - it('...failed', fakeAsync(() => { + it('...failed', () => { spyOn(router, 'createUrlTree').and.callThrough(); spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', { a: remoteDataMocks.RequestPending, b: remoteDataMocks.ResponsePending, c: remoteDataMocks.Error, })); - resolver(route, state, bitstreamDataService, serverResponseService, router).subscribe(() => { + resolver(route, state, bitstreamDataService, hardRedirectService, router).subscribe(() => { expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled(); expect(router.createUrlTree).toHaveBeenCalledWith([PAGE_NOT_FOUND_PATH]); }); - })); + }); - it('...succeeded without content', fakeAsync(() => { + it('...succeeded without content', () => { spyOn(router, 'createUrlTree').and.callThrough(); spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', { a: remoteDataMocks.RequestPending, b: remoteDataMocks.ResponsePending, c: remoteDataMocks.NoContent, })); - resolver(route, state, bitstreamDataService, serverResponseService, router).subscribe(() => { + resolver(route, state, bitstreamDataService, hardRedirectService, router).subscribe(() => { expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled(); expect(router.createUrlTree).toHaveBeenCalledWith([PAGE_NOT_FOUND_PATH]); }); - })); + }); - it('...succeeded', fakeAsync(() => { - spyOn(serverResponseService, 'setStatus').and.callThrough(); + it('...succeeded', () => { + spyOn(hardRedirectService, 'redirect'); spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', { a: remoteDataMocks.RequestPending, b: remoteDataMocks.ResponsePending, c: remoteDataMocks.Success, })); - resolver(route, state, bitstreamDataService, serverResponseService, router).subscribe(() => { + resolver(route, state, bitstreamDataService, hardRedirectService, router).subscribe(() => { expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled(); - expect(serverResponseService.setStatus).toHaveBeenCalledWith(301); - expect(router.parseUrl).toHaveBeenCalled(); + expect(hardRedirectService.redirect).toHaveBeenCalledWith(new URL(`/bitstreams/${bitstream.uuid}/download`, window.location.origin).href, 301); }); - })); + }); }); }); }); diff --git a/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.ts b/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.ts index 6c6357b5a5..d8d8932c30 100644 --- a/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.ts +++ b/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.ts @@ -1,14 +1,21 @@ import { inject } from '@angular/core'; -import { ActivatedRouteSnapshot, CanActivateFn, UrlTree, Router, RouterStateSnapshot } from '@angular/router'; +import { + ActivatedRouteSnapshot, + CanActivateFn, + Router, + RouterStateSnapshot, + UrlTree, +} from '@angular/router'; import { Observable } from 'rxjs'; -import { RemoteData } from '../core/data/remote-data'; -import { Bitstream } from '../core/shared/bitstream.model'; -import { hasNoValue } from '../shared/empty.util'; -import { BitstreamDataService } from '../core/data/bitstream-data.service'; -import { ServerResponseService } from '../core/services/server-response.service'; -import { map, tap } from 'rxjs/operators'; +import { map } from 'rxjs/operators'; + import { PAGE_NOT_FOUND_PATH } from '../app-routing-paths'; +import { BitstreamDataService } from '../core/data/bitstream-data.service'; +import { RemoteData } from '../core/data/remote-data'; +import { HardRedirectService } from '../core/services/hard-redirect.service'; +import { Bitstream } from '../core/shared/bitstream.model'; import { getFirstCompletedRemoteData } from '../core/shared/operators'; +import { hasNoValue } from '../shared/empty.util'; /** * Redirects to a bitstream based on the handle of the item, and the sequence id or the filename of the @@ -21,9 +28,9 @@ export const legacyBitstreamURLRedirectGuard: CanActivateFn = ( route: ActivatedRouteSnapshot, state: RouterStateSnapshot, bitstreamDataService: BitstreamDataService = inject(BitstreamDataService), - serverResponseService: ServerResponseService = inject(ServerResponseService), + serverHardRedirectService: HardRedirectService = inject(HardRedirectService), router: Router = inject(Router), -): Observable => { +): Observable => { const prefix = route.params.prefix; const suffix = route.params.suffix; const filename = route.params.filename; @@ -37,14 +44,10 @@ export const legacyBitstreamURLRedirectGuard: CanActivateFn = ( filename, ).pipe( getFirstCompletedRemoteData(), - tap((rd: RemoteData) => { - if (rd.hasSucceeded && !rd.hasNoContent) { - serverResponseService.setStatus(301); - } - }), map((rd: RemoteData) => { if (rd.hasSucceeded && !rd.hasNoContent) { - return router.parseUrl(`/bitstreams/${rd.payload.uuid}/download`); + serverHardRedirectService.redirect(new URL(`/bitstreams/${rd.payload.uuid}/download`, serverHardRedirectService.getCurrentOrigin()).href, 301); + return false; } else { return router.createUrlTree([PAGE_NOT_FOUND_PATH]); } diff --git a/src/app/core/data/bitstream-data.service.ts b/src/app/core/data/bitstream-data.service.ts index bb4ec28166..bf8e3585df 100644 --- a/src/app/core/data/bitstream-data.service.ts +++ b/src/app/core/data/bitstream-data.service.ts @@ -171,7 +171,7 @@ export class BitstreamDataService extends IdentifiableDataService imp searchParams.push(new RequestParam('sequenceId', sequenceId)); } if (hasValue(filename)) { - searchParams.push(new RequestParam('filename', filename)); + searchParams.push(new RequestParam('filename', encodeURIComponent(filename))); } const hrefObs = this.getSearchByHref(