refactor: Make signed request the final transformation

[ninegua]

Upgrading to replica v0.5.4 broke userlib/js (#552), partly because content field was made explicit in http_handler. This PR fixes this problem by introducing HttpAgentRequest -> SignedHttpAgentRequest as the last step of transform pipeline. I think this makes sense because we don't want to accidentally modify the content after signing.

I'm putting it out as a draft to seek opinions. Manually testing hello canister from browser seems to work fine. But I've no idea why previous e2e test didn't catch the breakage introduced in #552.

[chenyan-dfinity]

In the hello canister, after entering the name, you should get back the alert window again with the name. I don't see the second alert window, and replica shows the following error:

Apr 10 00:51:03.621 INFO Failed to authenticate request 0x0bba898cc27a8845064f268ad2ccac4143e3494bdcff871ea6ae39a2bdadd4fa, Application: Http Handler

[ninegua]

It works for me now. However I'm getting the following errors in (firefox) JS console, despite that the dialog box popping up correctly showing the name I typed.

JavaScript warning: http://127.0.0.1:8080/index.js, line 2: TypeError: asm.js type error: expecting argument type declaration for 't' of the form 'arg = arg|0' or 'arg = +arg' or 'arg = fround(arg)'
JavaScript error: http://127.0.0.1:8080/index.js, line 2: Error: Invalid text argument: "null"
JavaScript warning: http://127.0.0.1:8080/index.js, line 2: TypeError: asm.js type error: expecting argument type declaration for 't' of the form 'arg = arg|0' or 'arg = +arg' or 'arg = fround(arg)'
JavaScript warning: http://127.0.0.1:8080/index.js, line 2: TypeError: asm.js type error: expecting argument type declaration for 't' of the form 'arg = arg|0' or 'arg = +arg' or 'arg = fround(arg)'
JavaScript error: http://127.0.0.1:8080/index.js, line 2: Error: Invalid text argument: "null"

Is this something that I should be concerned with?

[chenyan-dfinity]

Thanks for doing this! I tried a bunch of canisters, they all work well! The asm.js warning is always there, I don't know where they come from.

[hansl]

They come from TweetNACL which has some unformed ASM.js.

There was a discussion when the pipeline was implemented to have the auth like you said. I made the case here for keeping the pipeline as is: #273 (review)

Long story short; it's better to leave the constructor of http_agent be flexible, and there are plenty of cases where we want transforms AFTER the signature. E.g. logging or breakpoint or splitting the request to a different backend, ....

So I would keep the pipeline as a list of steps and auth being one of them. There could be a warning that having auth not be the last step is risky (I think there's already a comment).

[hansl]

Spacing. Not sure why prettier didn't catch this.

[ninegua]

Which prettier was used? I'd like to set it up in my editor too.

[ninegua]

Ah, found npx prettier

[hansl]

It's supposed to run on CI too and fail if you didn't run it. Dunno why it didn't run :(

[ninegua]

This line npx prettier --check src/**/*.ts bootstrap/**/*.ts in package.json is wrong:

[nix-shell:~/dfinity/sdk/src/userlib/js]$ ls src/**/*ts
src/candid/candid-core.d.ts  src/candid/candid-ui.d.ts  src/utils/hash.d.ts       src/utils/hash.test.ts  src/utils/leb128.d.ts       src/utils/leb128.test.ts
src/candid/candid-core.ts    src/candid/candid-ui.ts    src/utils/hash.test.d.ts  src/utils/hash.ts       src/utils/leb128.test.d.ts  src/utils/leb128.ts

[ninegua]

Ok, the problem is that shopt -s globstar has to be enabled before we use **

[chenyan-dfinity]

Thanks for catching this. I will fix it in #558

[ninegua]

So I would keep the pipeline as a list of steps and auth being one of them.

I tried to keep it that way but due to the changes in data type, namely pubkey/signature now have to exist at an outer level, I couldn't find a clean approach to allow HttpAgentRequestTransformFn process both signed and unsigned requests. You are welcome to give it a try.

[ninegua]

there are plenty of cases where we want transforms AFTER the signature

If we keep things typed as signed vs. unsigned, then there are three stages:

1. HttpAgentRequest -> HttpAgentRequest
2. HttpAgentRequest -> SignedHttpAgentRequest
3. SignedHttpAgentRequest -> SignedHttpAgentRequest

We can use function composition to connect the pipeline, instead of setting priorities.

[hansl]

Yeah I think we should use a function composition. I can take care of it later. LGTM for now.