Skip to content

Commit

Permalink
fix(client-utils): only call fetch as a function, not a method (#10671
Browse files Browse the repository at this point in the history
)

closes: #XXXX
refs: endojs/endo#31 (comment)

## Description

Testing on Brave, the browser's global `fetch` function works when called with its `this` bound to either `undefined` or the browser global object, but does not work if its `this` is bound to something else. The code this PR fixes was misbehaving because it was calling `powers.fetch(...)`, i.e., happening to call it as a method with its `this` bound to the irrelevant `powers` object.

This refactor just expresses the intention of this code, which is just to call the `fetch` function without passing it some object as its `this` binding. This seems to fix the problem.

Not at all addressed by this PR: The fact that this problem happened from a programming patterns that seems correct and would have passed review indicates a deeper hazard. This PR does nothing to mitigate this deeper hazard. It only fixes this case where we fell into the hazard.

### Security Considerations

none

### Scaling Considerations
none

### Documentation Considerations
none, except as needed to address the more general hazard.

### Testing Considerations
Only tested `fetch` behavior on Brave. Based on that, proceeding under untested assumption that this refactor will work on all supported browsers.

### Upgrade Considerations
none
  • Loading branch information
erights authored Dec 11, 2024
1 parent 3b478fb commit fbae24c
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 4 deletions.
4 changes: 2 additions & 2 deletions a3p-integration/proposals/z:acceptance/test-lib/rpc.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ export { networkConfig };
* @param {typeof window.fetch} powers.fetch
* @param {MinimalNetworkConfig} config
*/
export const makeVStorage = (powers, config = networkConfig) => {
export const makeVStorage = ({ fetch }, config = networkConfig) => {
/** @param {string} path */
const getJSON = path => {
const url = config.rpcAddrs[0] + path;
// console.warn('fetching', url);
return powers.fetch(url, { keepalive: true }).then(res => res.json());
return fetch(url, { keepalive: true }).then(res => res.json());
};
// height=0 is the same as omitting height and implies the highest block
const url = (path = 'published', { kind = 'children', height = 0 } = {}) =>
Expand Down
4 changes: 2 additions & 2 deletions packages/client-utils/src/vstorage.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@
* @param {typeof window.fetch} powers.fetch
* @param {MinimalNetworkConfig} config
*/
export const makeVStorage = (powers, config) => {
export const makeVStorage = ({ fetch }, config) => {
/** @param {string} path */
const getJSON = path => {
const url = config.rpcAddrs[0] + path;
// console.warn('fetching', url);
return powers.fetch(url, { keepalive: true }).then(res => res.json());
return fetch(url, { keepalive: true }).then(res => res.json());
};
// height=0 is the same as omitting height and implies the highest block
const url = (path = 'published', { kind = 'children', height = 0 } = {}) =>
Expand Down

0 comments on commit fbae24c

Please sign in to comment.