From f1000f57f11b4e50d08e9bcb09e6b33a5297f84e Mon Sep 17 00:00:00 2001 From: Thomas Jung Date: Tue, 1 Sep 2026 20:39:14 -0400 Subject: [PATCH] fix(approuter): move /search 301 to middleware; drop invalid xs-app.json status prop PR #2099 added a `"status": 301` property to the /search route in xs-app.json. @sap/approuter v16 has no route-level redirect verb and rejects the unknown property at boot (xs-app.json/routes/52/status: Additional properties not allowed), crash-looping tutorials-dev-approuter (0/1). Fix mirrors the established sitemap-index-redirect.js pattern: - remove the invalid route from xs-app.json (the deeper /search/ srv-api proxy route is untouched) - add approuter/lib/search-redirect.js: a 301 middleware in insertMiddleware.first, before the static/proxy handlers, that redirects the exact /search entry point to /tutorial-navigator/ preserving the query - register it in server.js after sitemapIndexRedirectHandler Verified against @sap/approuter's own validators.validateXsApp: origin/DEV fails at routes/52/status; this commit PASSES. 10 new unit tests green. --- approuter/lib/search-redirect.js | 82 ++++++++++++++++++++++++++ approuter/server.js | 2 + approuter/xs-app.json | 1 - test/unit/search-redirect.test.js | 95 +++++++++++++++++++++++++++++++ 4 files changed, 179 insertions(+), 1 deletion(-) create mode 100644 approuter/lib/search-redirect.js create mode 100644 test/unit/search-redirect.test.js diff --git a/approuter/lib/search-redirect.js b/approuter/lib/search-redirect.js new file mode 100644 index 000000000..ec41881da --- /dev/null +++ b/approuter/lib/search-redirect.js @@ -0,0 +1,82 @@ +// approuter/lib/search-redirect.js +// +// 301-redirects the legacy `/search` entry point to this platform's canonical +// tutorial finder at /tutorial-navigator/, preserving any query string. +// +// Background: +// The old site exposed a top-level /search page. That has been replaced by +// /tutorial-navigator/ (an SSR page served from HANA via CAP). External links +// and bookmarks to /search (and /search?q=…) must land on the navigator, with +// the user's query carried across so the navigator can pre-seed its filter. +// +// Only the EXACT /search entry point redirects here — deeper /search/ +// URLs (e.g. the search JSON API) still proxy to srv-api via xs-app.json. +// +// Why middleware, not an xs-app.json route: +// @sap/approuter route config only rewrites+proxies or serves a localDir — it +// has no native 3xx-redirect verb. A prior attempt added `"status": 301` to an +// xs-app.json route; @sap/approuter v16 rejects that unknown property at boot +// (`xs-app.json/routes//status: Additional properties not allowed`), which +// crash-loops the approuter. A middleware in insertMiddleware.first is the same +// deterministic pattern used by sitemap-index-redirect.js / security-txt.js and +// runs BEFORE xs-app.json route matching, so /search/ (which this does +// NOT match) still proxies to srv-api untouched. +// +// Pure matcher + handler, both exported for the unit test. Approuter-native CJS +// (like sitemap-index-redirect.js) — NOT copied from srv/lib. + +'use strict' + +// The exact legacy search entry point at the site root: /search or /search/, +// optionally followed by a query string. Deliberately does NOT match deeper +// /search/ URLs so those keep proxying to srv-api. +const SEARCH_ENTRY_RE = /^\/search\/?$/ + +const NAVIGATOR_PATH = '/tutorial-navigator/' + +/** + * Map the legacy /search entry point to /tutorial-navigator/, preserving the + * query string. + * + * /search → /tutorial-navigator/ + * /search/ → /tutorial-navigator/ + * /search?q=cap → /tutorial-navigator/?q=cap + * /search/?q=cap → /tutorial-navigator/?q=cap + * + * Returns null for anything else — notably /search/ (the search API), + * which must keep proxying to srv-api. + * + * @param {string} url - path-or-path+query URL, e.g. '/search?q=cap' + * @returns {string | null} the navigator target for the /search entry point, else null + */ +function matchSearchUrl(url) { + if (typeof url !== 'string' || url.length === 0) return null + const qIdx = url.indexOf('?') + const pathname = qIdx === -1 ? url : url.slice(0, qIdx) + if (!SEARCH_ENTRY_RE.test(pathname)) return null + const query = qIdx === -1 ? '' : url.slice(qIdx) // includes the leading '?' + return NAVIGATOR_PATH + query +} + +// Express-style middleware. Mount at path '/' in insertMiddleware.first, BEFORE +// the static/proxy handlers so /search is answered here and never falls through +// to the xs-app.json /search/ proxy route. +function searchRedirectHandler(req, res, next) { + if (req.method !== 'GET' && req.method !== 'HEAD') return next() + + const target = matchSearchUrl(req.url || '') + if (!target) return next() + + res.writeHead(301, { + Location: target, + 'Cache-Control': 'public, max-age=86400', + }) + res.end() +} + +module.exports = { + searchRedirectHandler, + // exported for the unit test + matchSearchUrl, + NAVIGATOR_PATH, +} diff --git a/approuter/server.js b/approuter/server.js index 53dddde65..ecd775f22 100644 --- a/approuter/server.js +++ b/approuter/server.js @@ -25,6 +25,7 @@ const { mcpManifestHandler } = require('./lib/well-known-mcp-manifest') const { mcpAuthChallengeHandler } = require('./lib/mcp-auth-challenge') const { securityTxtHandler } = require('./lib/security-txt') const { sitemapIndexRedirectHandler } = require('./lib/sitemap-index-redirect') +const { searchRedirectHandler } = require('./lib/search-redirect') const shouldProcessImage = require('./lib/img-cdn-should-process') const { buildImageOriginUrl } = require('./lib/img-cdn-origin') const { ImgCache } = require('./lib/img-cdn-cache') @@ -597,6 +598,7 @@ ar.start({ { path: '/', handler: mcpManifestHandler }, { path: '/', handler: securityTxtHandler }, { path: '/', handler: sitemapIndexRedirectHandler }, + { path: '/', handler: searchRedirectHandler }, { path: '/', handler: mcpAuthChallengeHandler }, { path: '/', handler: devtoberfestCspHandler }, { path: '/', handler: imgCdnHandler }, diff --git a/approuter/xs-app.json b/approuter/xs-app.json index c1128a70a..a802fa8ea 100644 --- a/approuter/xs-app.json +++ b/approuter/xs-app.json @@ -371,7 +371,6 @@ "destination": "srv-api", "authenticationType": "xsuaa" }, - { "source": "^/search/?(\\?.*)?$", "target": "/tutorial-navigator/$1", "status": 301, "authenticationType": "none" }, { "source": "^/search/(.*)$", "target": "/search/$1", diff --git a/test/unit/search-redirect.test.js b/test/unit/search-redirect.test.js new file mode 100644 index 000000000..5bb34e455 --- /dev/null +++ b/test/unit/search-redirect.test.js @@ -0,0 +1,95 @@ +import { describe, it, expect } from 'vitest' +import { createRequire } from 'node:module' + +// Approuter-native CJS module (like sitemap-index-redirect / security-txt). +const require = createRequire(import.meta.url) +const { + searchRedirectHandler, + matchSearchUrl, + NAVIGATOR_PATH, +} = require('../../approuter/lib/search-redirect.js') + +// Minimal (req, res, next) mock, same shape as sitemap-index-redirect.test.js. +function mockRes() { + return { + statusCode: null, + headers: null, + body: null, + writeHead(status, headers) { this.statusCode = status; this.headers = headers; return this }, + end(payload) { this.body = payload; return this }, + } +} + +describe('matchSearchUrl', () => { + it('maps the bare /search entry point to the navigator', () => { + expect(matchSearchUrl('/search')).toBe('/tutorial-navigator/') + expect(matchSearchUrl('/search/')).toBe('/tutorial-navigator/') + }) + + it('preserves the query string', () => { + expect(matchSearchUrl('/search?q=cap')).toBe('/tutorial-navigator/?q=cap') + expect(matchSearchUrl('/search/?q=cap')).toBe('/tutorial-navigator/?q=cap') + expect(matchSearchUrl('/search?q=a&tag=b')).toBe('/tutorial-navigator/?q=a&tag=b') + }) + + it('does NOT match deeper /search/ URLs (those proxy to srv-api)', () => { + expect(matchSearchUrl('/search/foo')).toBeNull() + expect(matchSearchUrl('/search/tutorials.json')).toBeNull() + expect(matchSearchUrl('/search/?q=cap'.replace('/?', '/x?'))).toBeNull() // '/searchx?...' + }) + + it('ignores unrelated / lookalike paths', () => { + expect(matchSearchUrl('/searching')).toBeNull() + expect(matchSearchUrl('/foo/search')).toBeNull() + expect(matchSearchUrl('/tutorials-qa/search')).toBeNull() + }) + + it('handles non-string / empty input defensively', () => { + expect(matchSearchUrl('')).toBeNull() + expect(matchSearchUrl(undefined)).toBeNull() + expect(matchSearchUrl(null)).toBeNull() + }) +}) + +describe('searchRedirectHandler — approuter middleware', () => { + it('301s /search to the navigator with a 1-day cache', () => { + const res = mockRes() + let nexted = false + searchRedirectHandler({ method: 'GET', url: '/search', headers: {} }, res, () => { nexted = true }) + expect(nexted).toBe(false) + expect(res.statusCode).toBe(301) + expect(res.headers.Location).toBe(NAVIGATOR_PATH) + expect(res.headers['Cache-Control']).toMatch(/max-age=86400/) + expect(res.body).toBeUndefined() + }) + + it('301s /search?q=cap preserving the query', () => { + const res = mockRes() + searchRedirectHandler({ method: 'GET', url: '/search?q=cap', headers: {} }, res, () => {}) + expect(res.statusCode).toBe(301) + expect(res.headers.Location).toBe('/tutorial-navigator/?q=cap') + }) + + it('answers HEAD with the same 301', () => { + const res = mockRes() + searchRedirectHandler({ method: 'HEAD', url: '/search', headers: {} }, res, () => {}) + expect(res.statusCode).toBe(301) + expect(res.headers.Location).toBe(NAVIGATOR_PATH) + }) + + it('passes /search/ through untouched (proxied to srv-api downstream)', () => { + const res = mockRes() + let nexted = false + searchRedirectHandler({ method: 'GET', url: '/search/tutorials.json', headers: {} }, res, () => { nexted = true }) + expect(nexted).toBe(true) + expect(res.statusCode).toBeNull() + }) + + it('passes through non-GET/HEAD methods', () => { + const res = mockRes() + let nexted = false + searchRedirectHandler({ method: 'POST', url: '/search', headers: {} }, res, () => { nexted = true }) + expect(nexted).toBe(true) + expect(res.statusCode).toBeNull() + }) +})