Skip to content
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

REQUEST now supports POST #588

Merged
merged 8 commits into from
Jul 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 README.md
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ GRIST_DEFAULT_PRODUCT | if set, this controls enabled features and limits of ne
GRIST_DEFAULT_LOCALE | Locale to use as fallback when Grist cannot honour the browser locale.
GRIST_DOMAIN | in hosted Grist, Grist is served from subdomains of this domain. Defaults to "getgrist.com".
GRIST_EXPERIMENTAL_PLUGINS | enables experimental plugins
GRIST_ENABLE_REQUEST_FUNCTION | enables the REQUEST function. This function performs HTTP requests in a similar way to `requests.request`. This function presents a significant security risk, since it can let users call internal endpoints when Grist is available publicly. This function can also cause performance issues. Unset by default.
GRIST_HIDE_UI_ELEMENTS | comma-separated list of UI features to disable. Allowed names of parts: `helpCenter,billing,templates,multiSite,multiAccounts,sendToDrive,tutorials`. If a part also exists in GRIST_UI_FEATURES, it will still be disabled.
GRIST_HOME_INCLUDE_STATIC | if set, home server also serves static resources
GRIST_HOST | hostname to use when listening on a port.
Expand Down
2 changes: 2 additions & 0 deletions app/common/ActionBundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ export interface SandboxActionBundle {
// Represents a unique call to the Python REQUEST function
export interface SandboxRequest {
url: string;
method: string;
body?: string;
params: Record<string, string> | null;
headers: Record<string, string> | null;
deps: unknown; // pass back to the sandbox unchanged in the response
Expand Down
5 changes: 5 additions & 0 deletions app/server/devServerMain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ export async function main() {
process.env.GRIST_EXPERIMENTAL_PLUGINS = "1";
}

// Experimental plugins are enabled by default for devs
if (!process.env.GRIST_ENABLE_REQUEST_FUNCTION) {
process.env.GRIST_ENABLE_REQUEST_FUNCTION = "1";
}

// For tests, it is useful to start with the database in a known state.
// If TEST_CLEAN_DATABASE is set, we reset the database before starting.
if (process.env.TEST_CLEAN_DATABASE) {
Expand Down
11 changes: 8 additions & 3 deletions app/server/lib/Requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,21 @@ export class DocRequests {

private async _handleSingleRequestRaw(request: SandboxRequest): Promise<Response> {
try {
if (process.env.GRIST_EXPERIMENTAL_PLUGINS != '1') {
if (process.env.GRIST_ENABLE_REQUEST_FUNCTION != '1') {
throw new Error("REQUEST is not enabled");
}
const {url, params, headers} = request;
const {url, method, body, params, headers} = request;
const urlObj = new URL(url);
log.rawInfo("Handling sandbox request", {host: urlObj.host, docId: this._activeDoc.docName});
for (const [param, value] of Object.entries(params || {})) {
urlObj.searchParams.append(param, value);
}
const response = await fetch(urlObj.toString(), {headers: headers || {}, agent: proxyAgent(urlObj)});
const response = await fetch(urlObj.toString(), {
headers: headers || {},
agent: proxyAgent(urlObj),
method,
body
});
const content = await response.buffer();
const {status, statusText} = response;
const encoding = httpEncoding(response.headers.get('content-type'), content);
Expand Down
63 changes: 57 additions & 6 deletions sandbox/grist/functions/info.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
from __future__ import absolute_import
import datetime
import hashlib
import json
import json as json_module
import math
import numbers
import re

import chardet
import six
from six.moves import urllib_parse

import column
import docmodel
Expand Down Expand Up @@ -653,20 +654,70 @@ def is_error(value):
or (isinstance(value, float) and math.isnan(value)))


def _replicate_requests_body_args(data=None, json=None):
"""
Replicate some of the behaviour of requests.post, specifically the data and
johncant marked this conversation as resolved.
Show resolved Hide resolved
json args.

Returns a tuple of (body, extra_headers)
"""
if data is None and json is None:
return None, {}

elif data is not None and json is None:
if isinstance(data, str):
body = data
extra_headers = {}
else:
body = urllib_parse.urlencode(data)
extra_headers = {
"Content-Type": "application/x-www-form-urlencoded",
}
return body, extra_headers

elif json is not None and data is None:
if isinstance(json, str):
body = json
else:
body = json_module.dumps(json)
extra_headers = {
"Content-Type": "application/json",
}
return body, extra_headers

elif data is not None and json is not None:
# From testing manually with requests 2.28.2, data overrides json if both
# supplied. However, this is probably a mistake on behalf of the caller, so
# we choose to throw an error instead
raise ValueError("`data` and `json` cannot be supplied to REQUEST at the same time")


@unimplemented
# ^ This excludes this function from autocomplete while in beta
# and marks it as unimplemented in the docs.
# It also makes grist-help expect to see the string 'raise NotImplemented' in the function source,
# which it does now, because of this comment. Removing this comment will currently break the docs.
def REQUEST(url, params=None, headers=None):
# Makes a GET HTTP request with an API similar to `requests.get`.
def REQUEST(url, params=None, headers=None, method="GET", data=None, json=None):
# Makes an HTTP request with an API similar to `requests.request`.
# Actually jumps through hoops internally to make the request asynchronously (usually)
# while feeling synchronous to the formula writer.

# When making a POST or PUT request, REQUEST supports `data` and `json` args, from `requests.request`:
# - `args` as str: Used as the request body
# - `args` as other types: Form encoded and used as the request body. The correct header is also set.
# - `json` as str: Used as the request body. The correct header is also set.
# - `json` as other types: JSON encoded and set as the request body. The correct header is also set.
body, _headers = _replicate_requests_body_args(data=data, json=json)

# Extra headers that make us consistent with requests.post must not override
# user-supplied headers.
_headers.update(headers or {})

# Requests are identified by a string key in various places.
# The same arguments should produce the same key so the request is only made once.
args = dict(url=url, params=params, headers=headers)
args_json = json.dumps(args, sort_keys=True)
args = dict(url=url, params=params, headers=_headers, method=method, body=body)

args_json = json_module.dumps(args, sort_keys=True)
key = hashlib.sha256(args_json.encode()).hexdigest()

# This may either return the raw response data or it may raise a special exception
Expand Down Expand Up @@ -701,7 +752,7 @@ def text(self):
return self.content.decode(self.encoding)

def json(self, **kwargs):
return json.loads(self.text, **kwargs)
return json_module.loads(self.text, **kwargs)

@property
def ok(self):
Expand Down
44 changes: 43 additions & 1 deletion sandbox/grist/test_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import test_engine
import testutil
from functions import CaseInsensitiveDict, Response, HTTPError
from functions.info import _replicate_requests_body_args


class TestCaseInsensitiveDict(unittest.TestCase):
Expand Down Expand Up @@ -73,6 +74,45 @@ def test_apparent_encoding(self):
self.assertEqual(r.text, text)


class TestRequestsPostInterface(unittest.TestCase):
def test_no_post_args(self):
body, headers = _replicate_requests_body_args()

assert body is None
assert headers == {}

def test_data_as_dict(self):
body, headers = _replicate_requests_body_args(data={"foo": "bar"})

assert body == "foo=bar"
assert headers == {"Content-Type": "application/x-www-form-urlencoded"}

def test_data_as_string(self):
body, headers = _replicate_requests_body_args(data="some_content")

assert body == "some_content"
assert headers == {}

def test_json_as_dict(self):
body, headers = _replicate_requests_body_args(json={"foo": "bar"})

assert body == '{"foo": "bar"}'
assert headers == {"Content-Type": "application/json"}

def test_json_as_string(self):
body, headers = _replicate_requests_body_args(json="invalid_but_ignored")

assert body == "invalid_but_ignored"
assert headers == {"Content-Type": "application/json"}

def test_data_and_json_together(self):
with self.assertRaises(ValueError):
body, headers = _replicate_requests_body_args(
json={"foo": "bar"},
data={"quux": "jazz"}
)


class TestRequestFunction(test_engine.EngineTestCase):
sample = testutil.parse_test_sample({
"SCHEMA": [
Expand All @@ -98,12 +138,14 @@ def test_request_function(self):
r.__dict__
"""
out_actions = self.modify_column("Table1", "Request", formula=formula)
key = '9d305be9664924aaaf7ebb0bab2e4155d1fa1b9dcde53e417f1a9f9a2c7e09b9'
key = 'd7f8cedf177ab538bf7dadf66e77a525486a29a41ce4520b2c89a33e39095fed'
deps = {'Table1': {'Request': [1, 2]}}
args = {
'url': 'my_url',
'headers': {'foo': 'bar'},
'params': {'a': 2, 'b': 1},
'method': 'GET',
'body': None,
'deps': deps,
}
self.assertEqual(out_actions.requests, {key: args})
Expand Down