From 8e7e4c22419c6c1b74633771019708cc674a916a Mon Sep 17 00:00:00 2001 From: Lucas Werkmeister Date: Sat, 7 Sep 2024 15:07:12 +0200 Subject: [PATCH] Send POST requests using FormData if possible This partially walks back f6e28b57a7. 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 --- CHANGES.md | 2 ++ fetch.js | 15 ++++++-- test/unit/fetch.test.js | 78 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 3 deletions(-) create mode 100644 test/unit/fetch.test.js diff --git a/CHANGES.md b/CHANGES.md index 7473a95..9a0b83e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -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) diff --git a/fetch.js b/fetch.js index 2b2a1e2..5248116 100644 --- a/fetch.js +++ b/fetch.js @@ -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 ); } diff --git a/test/unit/fetch.test.js b/test/unit/fetch.test.js new file mode 100644 index 0000000..18350ce --- /dev/null +++ b/test/unit/fetch.test.js @@ -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 ] ); + } ); + + } ); + +} );