Skip to content

Commit

Permalink
Send POST requests using FormData if possible
Browse files Browse the repository at this point in the history
This partially walks back f6e28b5. It turns out that the type of the
fetch body affects the Content-Type sent with the request:
URLSearchParams corresponds to application/x-www-form-urlencoded and
FormData to multipart/form-data. For a simple request, the former looks
like this:

    Content-Type: application/x-www-form-urlencoded;charset=UTF-8

    meta=siteinfo&format=json

Whereas the latter looks like this:

    Content-Type: multipart/form-data; boundary=---------------------------384998708438653208232613793789

    -----------------------------384998708438653208232613793789
    Content-Disposition: form-data; name="meta"

    siteinfo
    -----------------------------384998708438653208232613793789
    Content-Disposition: form-data; name="format"

    json
    -----------------------------384998708438653208232613793789--

So using URLSearchParams is already much more efficient, resulting in
fewer bytes being sent over the wire (though in most m3api applications
the majority of traffic will be received from the server anyway; and
note that this only applies to POST requests anyway, not GET requests).

Additionally, it turns out that OAuth 2.0 requires requests to the
access token endpoint to be sent using application/x-www-form-urlencoded
(according to section 4.1.3 of RFC 6749 [1]), and recently MediaWiki
started to enforce this [2], which meant that m3api-oauth2 was broken
for Wikimedia production with all m3api versions since 0.8.0. Using
URLSearchParams when possible fixes lucaswerkmeister/m3api-oauth2/#3.

[1]: https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.3
[2]: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/OAuth/+/1046715
  • Loading branch information
lucaswerkmeister committed Sep 7, 2024
1 parent af2b9a2 commit 8e7e4c2
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 3 deletions.
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ but this file may sometimes contain later improvements (e.g. typo fixes).

- Updated the link to the [User-Agent policy][]
after it was moved from metawiki to foundationwiki.
- Optimized the internal encoding of POST requests;
this also fixes [m3api-oauth2][] when targeting wikis running MediaWiki 1.43 or later.

## v0.8.2 (2024-04-09)

Expand Down
15 changes: 12 additions & 3 deletions fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,23 @@ class FetchSession extends Session {
async internalPost( apiUrl, urlParams, bodyParams, headers ) {
const url = new URL( apiUrl );
url.search = new URLSearchParams( urlParams );
const body = new FormData();
// try to send the body as application/x-www-form-urlencoded (URLSearchParams),
// as this is required for OAuth 2.0 and also shorter;
// fall back to multipart/form-data (FormData) if needed for e.g. file params
let body1 = new URLSearchParams();
const body2 = new FormData();
for ( const [ paramName, paramValue ] of Object.entries( bodyParams ) ) {
body.append( paramName, paramValue );
if ( body1 !== null && typeof paramValue === 'string' ) {
body1.append( paramName, paramValue );
} else {
body1 = null;
}
body2.append( paramName, paramValue );
}
const response = await fetch( url, {
...this.getFetchOptions( headers ),
method: 'POST',
body,
body: body1 !== null ? body1 : body2,
} );
return transformResponse( response );
}
Expand Down
78 changes: 78 additions & 0 deletions test/unit/fetch.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/* eslint-env mocha */

import { set } from '../../core.js';
import { FetchSession } from '../../fetch.js';
import { expect } from 'chai';
import { File } from 'buffer'; // only available globally since Node 20

describe( 'FetchSession', () => {

let realFetch, mockFetchArguments;

before( function storeFetch() {
realFetch = global.fetch;
} );

beforeEach( function mockFetch() {
global.fetch = async function ( ...args ) {
mockFetchArguments = args;
return new Response( '{}' );
};
} );

afterEach( function unmockFetch() {
global.fetch = realFetch;
mockFetchArguments = undefined;
} );

const session = new FetchSession(
'en.wikipedia.org',
{},
{ userAgent: 'm3api-unit-test' },
);

describe( 'internalPost', () => {

it( 'uses URLSearchParams if possible', async () => {
await session.request( {
action: 'query',
meta: set( 'userinfo', 'siteinfo' ),
siprop: [ 'general', 'namespaces' ], // array instead of set just for coverage
curtimestamp: true,
formatversion: 2,
}, { method: 'POST' } );
expect( mockFetchArguments ).to.have.lengthOf( 2 );
expect( mockFetchArguments[ 0 ] ).to.be.an.instanceof( URL );
expect( mockFetchArguments[ 0 ].toString() ).to.equal(
'https://en.wikipedia.org/w/api.php?action=query',
);
expect( mockFetchArguments[ 1 ] ).to.have.deep.property( 'headers', {
'user-agent': session.getUserAgent( {} ),
} );
expect( mockFetchArguments[ 1 ] ).to.have.property( 'method', 'POST' );
expect( mockFetchArguments[ 1 ].body ).to.be.an.instanceof( URLSearchParams );
expect( mockFetchArguments[ 1 ].body.toString() ).to.equal(
new URLSearchParams( {
meta: 'userinfo|siteinfo',
siprop: 'general|namespaces',
curtimestamp: '',
formatversion: '2',
format: 'json',
} ).toString(),
);
} );

it( 'uses FormData if necessary', async () => {
const file = new File( [ '1' ], { type: 'text/plain' } );
await session.request( {
file,
}, { method: 'POST' } );
expect( mockFetchArguments[ 1 ].body ).to.be.an.instanceof( FormData );
expect( [ ...mockFetchArguments[ 1 ].body.keys() ] ).to.eql( [ 'file', 'format' ] );
expect( mockFetchArguments[ 1 ].body.getAll( 'format' ) ).to.eql( [ 'json' ] );
expect( mockFetchArguments[ 1 ].body.getAll( 'file' ) ).to.eql( [ file ] );
} );

} );

} );

1 comment on commit 8e7e4c2

@lucaswerkmeister
Copy link
Owner Author

Choose a reason for hiding this comment

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

lucaswerkmeister/m3api-oauth2/#3

Oops, make that lucaswerkmeister/m3api-oauth2#3.

Please sign in to comment.