Skip to content

[MMM] Fix 500 Internal Server Error when toggling manifest drawer #5801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
markpadbe wants to merge 1 commit into
base: stage
Choose a base branch
from
Open

[MMM] Fix 500 Internal Server Error when toggling manifest drawer #5801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions libs/blocks/mmm/mmm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ async function toggleDrawer(target, dd, pageId) {
const loading = dd.querySelector('.loading');
if (dd.classList.contains('placeholder-resolved') || !loading) return;
const pageData = await fetchData(`${API_URLS.pageDetails}?id=${pageId}&lastSeen=${SEARCH().lastSeenManifest}&manifestSrc=${SEARCH().manifestSrc}`, DATA_TYPE.JSON);
if (!pageData) return;
loading.replaceWith(await getMepPopup(pageData, true));
dd.classList.add('placeholder-resolved');
}
Expand Down
13 changes: 10 additions & 3 deletions libs/features/personalization/personalization.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,13 @@ export const normalizePath = (p, localize = true) => {
const firstFolder = pathname.split('/')[1];
const mepHash = '#_dnt';

if (path.startsWith(config.codeRoot)
const isKnownOrigin = path.startsWith(config.codeRoot)
|| path.includes('.hlx.')
|| path.includes('.aem.')
|| path.includes('.adobe.')
|| path.includes('localhost:')) {
|| path.includes('localhost:');

if (isKnownOrigin) {
if (!localize
|| config.locale?.ietf === 'en-US'
|| hash?.includes(mepHash)
Expand All @@ -124,7 +126,12 @@ export const normalizePath = (p, localize = true) => {
}
}
path = isFederal ? getFederatedUrl(path) : path;
return `${path}${search}${hash.replace(mepHash, '')}`;
if (isKnownOrigin) {
return `${path}${search}${hash.replace(mepHash, '')}`;
Copy link
Copy Markdown
Contributor

@denlight denlight Apr 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we are addressing the case where path has search already. if it does, we are going to append search again, which is what is breaking mmm currently. I believe what was meant to be used is pathname?

Copy link
Copy Markdown
Contributor Author

@markpadbe markpadbe Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. This line (130) only runs when isKnownOrigin is true, which means path has already been reassigned to just pathname on line 121. So appending ${search} here is re-attaching the query params to the stripped pathname, not duplicating them.

The case you're describing (where path retains the full URL) is handled just below on lines 132-134, in the !isKnownOrigin branch. There we reconstruct via new URL(path) which preserves existing query params without appending anything.

}
const normalizedUrl = new URL(path);
normalizedUrl.hash = normalizedUrl.hash.replace(mepHash, '');
return normalizedUrl.toString();
} catch (e) {
path = isFederal ? getFederatedUrl(path) : path;
return path;
Expand Down
16 changes: 16 additions & 0 deletions test/features/personalization/normalizePath.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,20 @@ describe('normalizePath function', () => {
expect(path).to.include('/federal/globalnav/acom/fragments/promos/test-promo');
expect(path).not.to.include('/de/');
});

it('does not duplicate query params for external URLs', () => {
config.locale = { ietf: 'en-US', prefix: '' };
const apiUrl = 'https://example.lambda-url.us-west-2.on.aws/get-page?id=123&lastSeen=week&manifestSrc=pzn,%20promo';
const result = normalizePath(apiUrl);
expect(result).to.equal(apiUrl);
expect(result.match(/id=123/g)).to.have.lengthOf(1);
});

it('strips #_dnt from external URLs without duplicating params', () => {
config.locale = { ietf: 'en-US', prefix: '' };
const apiUrl = 'https://example.lambda-url.us-west-2.on.aws/get-page?id=123#_dnt';
const result = normalizePath(apiUrl);
expect(result).to.equal('https://example.lambda-url.us-west-2.on.aws/get-page?id=123');
expect(result).to.not.include('#_dnt');
});
});
Loading