-
Notifications
You must be signed in to change notification settings - Fork 161
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
Support Web APIs. #299
Comments
There are a ton of web standards. ECMAScript® 2020 Language Specification (JavaScript) is just one of the many. DOM is another example. They are related but otherwise orthogonal.
https://developer.mozilla.org/en-US/docs/Web/API/Response/Response#Browser_compatibility BTW, every feature is marked as "Experimental. Expect behavior to change in the future." So we probably should wait a bit until dust is settled.
Take a look at an example. |
Take a look at an example of
We've talked about the usage of lua-module, that developers can write async-operations in sync way.
Lua has a mechanism called coroutine. See an example.
Actually, each request is wrapped as a coroutine function, so lua-module can do the async things like The purpose of
What about starting to implement
|
Hi!
Take a look at this example. |
Among Web APIs, the most interesting and promising is Web Crypto API. |
|
yes, ideally we want to use the same OpenSSL-like library which nginx is using. The trick is to get the right library (including the the libs that were provided as As on now njs configure does not see this information when it is built as a part of nginx (https://github.com/nginx/njs/blob/master/nginx/config.make). |
I am thinking about the way to split-apart byte strings and ordinary strings (http://nginx.org/en/docs/njs/reference.html#string), Byte-string - a sequence of bytes. Historically byte string were introduced as as thin (non-copy) wrappers around nginx native objects (for example r.requestBody, r.variables.hostname) and we want this property to stay. The problems arise when we work with byte-strings internally (concating ordinary and byte strings, joining array which contains byte strings), when byte-string cannot be represented as a valid UTF-8 string (for example a byte '0x9d' is not a valid UTF-8). There is also a backward compatibility issue, we should not change the current API too much. For example:
Ideally we want to get rid of byte-string altogether (replacing them with TypedArray).
I am thinking about some kind of Proxy object (similar to Blob). NjsBlob
NjsBlob.prototype.arrayBuffer()
NjsBlob.prototype.text() -> convert to UTF8 string()
NjsBlob.prototype.toString() -> text() // implicit conversion to String, but exception when it is not possible
JSON.stringify(r.requestBody) // works out of the box
r.requestBody.split('&') // requires changes, but seems manageable -> r.requestBody.text().split('&') NjsBlob != Blob - because the latter returns Promise(), seems too much overhead for simple things Possibly adding later StringView. What is your take on it? |
I like a
|
Yes, maybe implicit toString() is not a good idea. > var a = new Uint8Array([0x41, 0x42]); var b = new Blob([a.buffer]); await b.text()
"AB"
> b.toString()
"[object Blob]"
Buffer is not a standard, ES6 version looks like: const a = new Int8Array( [ 1, 2, 3 ] )
const b = new Int8Array( [ 4, 5, 6 ] )
const c = Int8Array.from([...a, ...b]) |
Looks so, the problem that it will partitialy work.
Yes, but it's well-known and it is a |
Maybe is better to use properties instead? NjsBlob
NjsBlob.prototype.size
NjsBlob.prototype.arrayBuffer
NjsBlob.prototype.text -> create/return a `string` representation |
Yes, am considering it. |
Meanwhile I am studying the way to port existing njs code automatically, it seems it is pretty straightforward in JS world. For example, to robustly rename in the code cat example.js
function jwt(data) {
return data.split('.').map(v=>String.
bytesFrom(v, 'base64url'));
} $ jscodeshift -p -d -t njs-0.5.0-api-transform.js example.js
Processing 1 files...
Spawning 1 workers...
Running in dry mode, no files will be written!
Sending 1 files to free worker...
The njs API transformer will do the following replacements:
Core:
String.bytesFrom() -> String.from()
function jwt(data) {
return data.split('.').map(v=>String.
from(v, 'base64url'));
}
All done.
Results:
0 errors
0 unmodified
0 skipped
1 ok
Time elapsed: 0.456seconds $ cat njs-0.5.0-api-transform.js
function transformer(file, api, options) {
const j = api.jscodeshift;
const source = j(file.source);
console.log(`
The njs API transformer will do the following replacements:
Core:
String.bytesFrom() -> String.from()
`);
/*
* String.bytesFrom() -> String.from();
*/
source
.find(j.CallExpression, {
callee: {
type: 'MemberExpression',
object: {
name: 'String'
},
property: {
name: 'bytesFrom'
}
}
})
.replaceWith(path => { path.value.callee.property.name = 'from'; return path.value;});
return source.toSource();
}
module.exports = transformer; |
I'm on the support of the native HTTP client.
Result
My thoughts are:
But it's overhead to implement
The
Welcome to your suggestions. |
https://gist.github.com/hongzhidao/d534f1536494ea87973cb49218bf1e9f
BTW, take a look at
As I understand,
|
Looks promising. Will take a look once I am with the Buffer object (most probably tomorrow). |
Sorry for my carelessness. This may copy the result one more time. It's good to use the same mem_pool. In the case of only one node, what about processing without allocating a new memory?
It's common that the client can receive a response body once, including chunked. |
In node.
In quickjs and njs, it works well. (It shows that Which way is correct? I'm wondering if it always returns NJS_OK if the vm event is
Since the |
Hi. Welcome to test
Patch. Now, It's only used in HTTP module since the stream module does not support async mechanism (guess). |
Hi!
But all of them are correct, 'cause:
|
@drsm
|
looks like browser version doesn't throw anything, even ngx.fetch("http://127.0.0.1:8080", {
method: "POST",
headers: {
"X-test": "xxxxx",
"Y-test": "yyyyy"
}
})
.then((rs) => {
// handle response
if (rs.status >= 400) {
var e = new Error(`fetch failed: ${rs.status}`);
e.response = rs;
throw e;
}
// ...
})
.catch((e) => {
// handle error
if (e.response) {
// server reports error
} else {
// timeout, DNS, etc...
}
}) |
Is this?
|
There |
@drsm
|
Yes, I think it would be nice to have some sugar like: r.response({ some: thing })
// ==
r.response(JSON.stringify({ some: thing }), { status: 200, headers: {'content-type': 'application/json', ...})
// ;
r.response(Buffer.from('some bytes'), ...) |
Hi guys, welcome to test Fetch API (ngx.fetch()). What is supported: http and stream modules, chunked encoding.
njs patch: https://gist.github.com/xeioex/f185ba904ee216e8490e20cf11c18e26 |
njs_fetch:
tests:
https://gist.github.com/xeioex/c224451823614d8701fb28d6a3290a8f#file-fetch_tests-patch-L119 BTW, will do more test after the New Year vacation :) |
Applied, thanks.
this is escaping for $ in perl string literals.
Good catches, thanks! |
ngx.fetch('http://104.28.2.142/r/hidden', { headers: { 'Host': 'requestbin.net' }})
.then((res) => typeof res.headers.get('XXX'))
.then((res) => r.return(200, res))
.catch((e) => r.return(500, e.message)); // failed to convert text
ret = ngx_response_js_ext_header_get(vm, njs_argument(args, 0),
&name, njs_vm_retval(vm));
if (ret == NJS_ERROR) {
return NJS_ERROR;
}
return NJS_OK; Happy New Year! |
@xeioex BTW, I noticed that we need to care about the exception and throw it obviously, then return error status, such as.
If we can unify them into njs_value. It would be clearer, like the following:
This's inspired by quickjs, most of the functions return with JSValue and passed JSValue parameters. |
Some issues I've found:
the
How about providing |
Not sure about disabling merging in |
yes, because in JS you can throw anything, like |
ngx.fetch() with DNS support.
njs patch: https://gist.github.com/98c4a9e8ae85c1a5a06669d7430de048 |
I found a problem, a duck disappear suddenly :) $ curl localhost:10000/test
h[x-duck] = 🦆
response:
H[Connection] = close
H[x-duck] = 🦆
H[Host] = 127.0.0.1
$ curl localhost:10000/test
h[x-duck] = 🦆
response:
H[Connection] = close
H[x-duck] = 🦆
H[Host] = 127.0.0.1
$ curl localhost:10000/test
h[x-duck] = null
response:
H[Connection] = close
H[x-duck] = 🦆
H[Host] = 127.0.0.1
function dump(r) {
var out = '';
for (var k in r.headersIn) {
out += `H[${k}] = ${r.headersIn[k]}\n`;
}
r.headersOut['x-duck'] = '🦆';
r.return(200, out);
}
function test(r) {
var out = '';
ngx.fetch('http://127.0.0.1:10000/dump', { headers: { 'x-duck' : '🦆' } })
.then((res) => {
out += `h[x-duck] = ${res.headers.get('x-duck')}\n`;
return res.text();
})
.then((res) => r.return(200, `${out}response:\n${res}\n`))
.catch((e) => r.return(500, e.message));
} |
Thanks:). The issue was trivial, I forgot to set a boolean value in a header struct. Failed to notice it because I prefer to run the nginx binary with address sanitizer enabled, which with high probability initialises a byte from dynamic memory with non-zero value. --- a/nginx/ngx_js_fetch.c
+++ b/nginx/ngx_js_fetch.c
@@ -1083,6 +1083,7 @@ ngx_js_http_process_headers(ngx_js_http_
return NGX_ERROR;
}
+ h->hash = 1;
h->key.data = hp->header_name_start;
h->key.len = hp->header_name_end - hp->header_name_start; |
Committed, thank you very much for your contribution. |
Hi all, I couldn't get from the discussion whether you're actually planning to add support of the Web APIs? Currently, we are building an automatic convertor of Cloudflare JS workers to NJS workers. Cloudflare workers support Web API 100%, so we ended up writing polyfills for Our
Please note, that some of the polyfills are already present in |
Hi @vldmri, We are planning to extend the WebAPI, but no specific plans yet.
|
Hi @xeioex
Is there any plan updated on the support of WebAPI? Thanks. |
Hi @hongzhidao, Yes, I am planning to add Request object support in the near-to-middle term future. |
Hi @hongzhidao, BTW, can you please elaborate why you need a Request and Response object? A good usecase where Request is necessary or makes code much more readable will be enough. |
Hi @xeioex , We are supporting njs in Unit. we aren't using it in the same way as nginx.
It's read-only. This is almost finished. Then we plan to support read-write actions like
It's not clear now, We'll make the decision clear internally first. Btw, you have a lot of experience in njs usage with nginx, your advice on how to use it in Unit will very useful. |
@vldmri Do you have any plans to open source the polyfills? We are thinking about an similiar implemenation for NGINX Unit while NJS is not supporting it natively. Just wanted to check in with you before starting any of this work. Thanks for your feedback. |
Hi @hongzhidao, you may be interested in nginx-testing and njs-typescript-starter. |
@tippexs you can use these polyfills. However, you need to compile it properly, we based our pipeline on @jirutka njs-typescript-starter. Also you may need request/response adapters https://www.npmjs.com/package/headers-polyfill |
I am aware of the typescript starter and modified a couple of things in my typescript setup! We should meet at our Nginx community slack and chat about it. Would look forward to it. Thanks for the links and I will have a look on it! |
@xeioex @drsm
Web APIs
is an alternative usage.https://developer.mozilla.org/en-US/docs/Web/API/Response/Response
It will behave in njs like below.
In the beginning, njs has individual response object.
foo(req, res)
. After that,res
is merged intoreq
, so we have to rename some methods like headersIn, headersOut.If we introduce these things,
Request
,Response
,Headers
,Body
, I believe it can be easier to use, and we just implement the standard methods with them.The second change is that the subrequest is also easier to use. Indeed, users need not know what
subrequest
is, they just want to send a request. (But the implementation will be upon on nginx subrequest).Still, further, it can be welcome after
async
,await
are introduced.Highly welcome to your suggestions. I'd like to try it without changing current usage by put them in
precontent
phase.I'm still not familiar with
async
,await
,Promise
, it's good to have a better example of how to write withsubrequest
below. @drsmBTW, do you know whether the above objects are standard belong to js?
I'm wondering which software has already implemented them, where are they used?
The text was updated successfully, but these errors were encountered: