Skip to content

Commit ec6bba5

Browse files
authored
fix: wrong construction of urlToUse leads to false alert with logger.error (#15190)
### What? Fixes operator precedence bug in `createLocalReq.ts` that causes false error logs when using Preview feature without a configured `serverURL`. ### Why The `||` operator has higher precedence than the ternary `?:`, causing incorrect URL construction logic. When `req.url` exists but `serverURL` is undefined, the code attempts to construct a URL from `undefined`, triggering an error log even though a valid URL is available. ### How Wrapped the ternary operator in parentheses to establish correct precedence: `req.url` is checked first, then falls back to `serverURL` construction if available, and finally uses localhost fallback. Fixes #12880
1 parent ef863e6 commit ec6bba5

File tree

2 files changed

+128
-2
lines changed

2 files changed

+128
-2
lines changed
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
import { describe, expect, it, vi } from 'vitest'
2+
3+
import type { Payload } from '../index.js'
4+
5+
import { createLocalReq } from './createLocalReq.js'
6+
7+
describe('createLocalReq - URL construction', () => {
8+
const mockPayload = {
9+
config: {
10+
serverURL: undefined,
11+
i18n: {
12+
fallbackLanguage: 'en',
13+
supportedLanguages: { en: {} },
14+
translations: {},
15+
},
16+
localization: undefined,
17+
},
18+
logger: {
19+
error: vi.fn(),
20+
},
21+
} as unknown as Payload
22+
23+
it('should use req.url when provided and serverURL is undefined', async () => {
24+
const req = {
25+
url: 'http://example.com/api/test',
26+
}
27+
28+
const result = await createLocalReq({ req }, mockPayload)
29+
30+
expect(result.url).toBe('http://example.com/api/test')
31+
expect(mockPayload.logger.error).not.toHaveBeenCalled()
32+
})
33+
34+
it('should use serverURL when req.url is not provided', async () => {
35+
const payloadWithServerURL = {
36+
config: {
37+
serverURL: 'http://configured-server.com',
38+
i18n: {
39+
fallbackLanguage: 'en',
40+
supportedLanguages: { en: {} },
41+
translations: {},
42+
},
43+
localization: undefined,
44+
},
45+
logger: {
46+
error: vi.fn(),
47+
},
48+
} as unknown as Payload
49+
50+
const req = {}
51+
52+
const result = await createLocalReq({ req, urlSuffix: '/api' }, payloadWithServerURL)
53+
54+
expect(result.url).toContain('http://configured-server.com/api')
55+
expect(payloadWithServerURL.logger.error).not.toHaveBeenCalled()
56+
})
57+
58+
it('should prioritize req.url over serverURL', async () => {
59+
const payloadWithServerURL = {
60+
config: {
61+
serverURL: 'http://configured-server.com',
62+
i18n: {
63+
fallbackLanguage: 'en',
64+
supportedLanguages: { en: {} },
65+
translations: {},
66+
},
67+
localization: undefined,
68+
},
69+
logger: {
70+
error: vi.fn(),
71+
},
72+
} as unknown as Payload
73+
74+
const req = {
75+
url: 'http://actual-request.com/api/test',
76+
}
77+
78+
const result = await createLocalReq({ req }, payloadWithServerURL)
79+
80+
expect(result.url).toBe('http://actual-request.com/api/test')
81+
expect(payloadWithServerURL.logger.error).not.toHaveBeenCalled()
82+
})
83+
84+
it('should fall back to localhost when neither req.url nor serverURL provided', async () => {
85+
const req = {}
86+
87+
const result = await createLocalReq({ req }, mockPayload)
88+
89+
expect(result.url).toBe('http://localhost/')
90+
expect(mockPayload.logger.error).not.toHaveBeenCalled()
91+
})
92+
93+
it('should append urlSuffix to serverURL when used', async () => {
94+
const payloadWithServerURL = {
95+
config: {
96+
serverURL: 'http://configured-server.com',
97+
i18n: {
98+
fallbackLanguage: 'en',
99+
supportedLanguages: { en: {} },
100+
translations: {},
101+
},
102+
localization: undefined,
103+
},
104+
logger: {
105+
error: vi.fn(),
106+
},
107+
} as unknown as Payload
108+
109+
const req = {}
110+
111+
const result = await createLocalReq({ req, urlSuffix: '/api/preview' }, payloadWithServerURL)
112+
113+
expect(result.url).toContain('/api/preview')
114+
expect(payloadWithServerURL.logger.error).not.toHaveBeenCalled()
115+
})
116+
117+
it('should append urlSuffix to fallback URL when neither req.url nor serverURL provided', async () => {
118+
const req = {}
119+
120+
const result = await createLocalReq({ req, urlSuffix: '/api/test' }, mockPayload)
121+
122+
expect(result.url).toBe('http://localhost/api/test')
123+
expect(mockPayload.logger.error).not.toHaveBeenCalled()
124+
})
125+
})

packages/payload/src/utilities/createLocalReq.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,10 @@ const attachFakeURLProperties = (req: Partial<PayloadRequest>, urlSuffix?: strin
3939
const fallbackURL = `http://${req.host || 'localhost'}${urlSuffix || ''}`
4040

4141
const urlToUse =
42-
req?.url || req.payload?.config?.serverURL
42+
req?.url ||
43+
(req.payload?.config?.serverURL
4344
? `${req.payload?.config.serverURL}${urlSuffix || ''}`
44-
: fallbackURL
45+
: fallbackURL)
4546

4647
try {
4748
urlObject = new URL(urlToUse)

0 commit comments

Comments
 (0)