Skip to content

Fix bug where v1 HTTP used v2 API if callback had len 1 #955

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.

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

Merged
merged 3 commits into from
Aug 16, 2021

Conversation

inlined
Copy link
Member

@inlined inlined commented Aug 16, 2021

The Callable API is complex enough that we share code between the v1 and v2 SDKs. The shared code sniffed the callback length to determine which API to surface, but this is a mistake because the callback is customer controlled and they may drop parameters they don't care about. By wrapping the customer-provided handler with a known-sized handler we can fix the sniffing behavior.

Fixes #947

@inlined inlined requested a review from taeold August 16, 2021 18:45
@inlined inlined force-pushed the inlined.https-callable-legacy branch from 6c79401 to bda0f10 Compare August 16, 2021 18:45
@google-cla google-cla bot added the cla: yes label Aug 16, 2021
@@ -175,6 +175,25 @@ describe('#onCall', () => {
};
expect(cf.run(data, context)).to.deep.equal({ data, context });
});

// Regression test for firebase-functions#947
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@inlined inlined force-pushed the inlined.https-callable-legacy branch from 28598ae to 1c42eed Compare August 16, 2021 20:56
@inlined inlined merged commit 78db0b8 into master Aug 16, 2021
@inlined inlined deleted the inlined.https-callable-legacy branch August 16, 2021 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

http.onCall now sends raw header and body in the first parameter
2 participants