Skip to content

Commit 69b5f1e

Browse files
authored
fix: Don't call function after 404 on /{robots.txt,favicon.ico} (#193)
Other changes: * Send an empty response body for the same with the 404, since these URLs are not intended for human consumption. * Add an invocation count assertion to the test to confirm that these URLs do not in fact result in the function call. * Accept npm's lock of this package version as test dependency. * Nit: wrap long comments at 80 characters. Fixes: #191
1 parent 4684d8b commit 69b5f1e

File tree

3 files changed

+21
-12
lines changed

3 files changed

+21
-12
lines changed

package-lock.json

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/invoker.ts

+8-7
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ function makeHttpHandler(execute: HttpFunction): express.RequestHandler {
123123
}
124124

125125
/**
126-
* Wraps cloudevent function (or cloudevent function with callback) in HTTP function
127-
* signature.
126+
* Wraps cloudevent function (or cloudevent function with callback) in HTTP
127+
* function signature.
128128
* @param userFunction User's function.
129129
* @return HTTP function which wraps the provided event function.
130130
*/
@@ -251,9 +251,10 @@ function registerFunctionRoutes(
251251
functionSignatureType: SignatureType
252252
) {
253253
if (functionSignatureType === SignatureType.HTTP) {
254-
app.use('/favicon.ico|/robots.txt', (req, res, next) => {
255-
res.sendStatus(404);
256-
next();
254+
app.use('/favicon.ico|/robots.txt', (req, res) => {
255+
// Neither crawlers nor browsers attempting to pull the icon find the body
256+
// contents particularly useful, so we send nothing in the response body.
257+
res.status(404).send(null);
257258
});
258259

259260
app.use('/*', (req, res, next) => {
@@ -372,8 +373,8 @@ export function getServer(
372373
req.rawBody = buf;
373374
}
374375

375-
// Set limit to a value larger than 32MB, which is maximum limit of higher level
376-
// layers anyway.
376+
// Set limit to a value larger than 32MB, which is maximum limit of higher
377+
// level layers anyway.
377378
const requestLimit = '1024mb';
378379
const defaultBodySavingOptions = {
379380
limit: requestLimit,

test/invoker.ts

+12-4
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ describe('request to HTTP function', () => {
2424
path: string;
2525
text: string;
2626
status: number;
27+
callCount: number;
2728
}
2829

2930
const testData: TestData[] = [
@@ -32,41 +33,48 @@ describe('request to HTTP function', () => {
3233
path: '/',
3334
text: 'HELLO',
3435
status: 200,
36+
callCount: 1,
3537
},
3638
{
3739
name: 'simple path',
3840
path: '/foo',
3941
text: 'HELLO',
4042
status: 200,
43+
callCount: 1,
4144
},
4245
{
4346
name: 'with favicon.ico',
4447
path: '/favicon.ico',
45-
text: 'Not Found',
48+
text: '',
4649
status: 404,
50+
callCount: 0,
4751
},
4852
{
4953
name: 'with robots.txt',
5054
path: '/robots.txt',
51-
text: 'Not Found',
55+
text: '',
5256
status: 404,
57+
callCount: 0,
5358
},
5459
];
5560

5661
testData.forEach(test => {
57-
it(`should return transformed body: ${test.name}`, () => {
62+
it(`should return transformed body: ${test.name}`, async () => {
63+
var callCount = 0;
5864
const server = invoker.getServer(
5965
(req: express.Request, res: express.Response) => {
66+
++callCount;
6067
res.send(req.body.text.toUpperCase());
6168
},
6269
invoker.SignatureType.HTTP
6370
);
64-
return supertest(server)
71+
await supertest(server)
6572
.post(test.path)
6673
.send({ text: 'hello' })
6774
.set('Content-Type', 'application/json')
6875
.expect(test.text)
6976
.expect(test.status);
77+
assert.strictEqual(callCount, test.callCount);
7078
});
7179
});
7280
});

0 commit comments

Comments
 (0)