scriptoriumadmin/zotero
1
0
Fork 0
mirror of https://github.com/zotero/zotero.git synced 2026-08-28 05:25:31 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Fix WebDAV authentication handling in fx140

Browse source 
Firefox no longer seems to send a previously used Authorization header
with subsequent requests. This results in extra requests, since every
WebDAV request triggers a 401, and also results in errors, because a PUT
is sent without Authorization, causing some WebDAV servers to
immediately send a 401 and close the connection, which the HTTP layer
interprets as a connection failure (status 0). (It's also not good to
try to send a large file just to get a 401.)

There might be some way to share context between requests, but instead,
just get the used Authorization header and include that explicitly in
future requests.

To test this properly, we have to switch to using httpd.js for all
WebDAV requests, since the mocked XHR doesn't trigger the 401 retry.

https://forums.zotero.org/discussion/129194/webdav-uploads-fail-on-zotero-8-put-sent-without-authorization-server-closes-connection
This commit is contained in:
Dan Stillman 2026-02-01 15:25:36 -05:00
parent e7d1772470
commit 089701eca8
3 changed files with 266 additions and 135 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
chrome/content/zotero/xpcom/http.js
View file

@ -136,6 +136,7 @@ Zotero.HTTP = new function () {
* @param {Function} [options.requestObserver] - Callback to receive XMLHttpRequest after open()
* @param {Function} [options.cancellerReceiver] - Callback to receive a function to cancel
* the operation
* @param {Function} [options.onAuthorizationHeader]
* @param {String} [options.responseType] - The type of the response. See XHR 2 documentation
* for legal values
* @param {String} [options.responseCharset] - The charset the response should be interpreted as
@ -464,6 +465,10 @@ Zotero.HTTP = new function () {
var status = redirectStatus || xmlhttp.status;
var success;
if (options.onAuthorizationHeader) {
options.onAuthorizationHeader(this.getChannelAuthorization(xmlhttp.channel));
}
try {
if (!status) {
let responseStatus = xmlhttp.channel.responseStatus;
@ -1249,7 +1254,7 @@ Zotero.HTTP = new function () {
return authHeader;
}
catch (e) {
Zotero.debug(e);
//Zotero.debug(e);
return false;
}
}

98
chrome/content/zotero/xpcom/storage/webdav.js
View file

@ -54,7 +54,15 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
_parentURI: null,
_rootURI: null,
_cachedCredentials: false,
_channelAuthorization: null,
_getAuthorizationHeaders() {
if (!this._channelAuthorization) {
Zotero.debug("Authorization header not cached");
return {};
}
return { Authorization: this._channelAuthorization };
},
_loginManagerHost: 'chrome://zotero',
_loginManagerRealm: 'Zotero Storage Server',
@ -118,7 +126,7 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
return;
}
_cachedCredentials = false;
this._channelAuthorization = false;
var logins = await Services.logins.searchLoginsAsync({
origin: this._loginManagerHost,
@ -229,7 +237,7 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
cacheCredentials: async function () {
await this._init();
if (this._cachedCredentials) {
if (this._channelAuthorization) {
Zotero.debug("WebDAV credentials are already cached");
return;
}
@ -244,11 +252,19 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
successCodes: [200, 204, 404],
errorDelayIntervals: this.ERROR_DELAY_INTERVALS,
errorDelayMax: this.ERROR_DELAY_MAX,
onAuthorizationHeader: (authorization) => {
// Capture whatever auth it ended up using, if any
this._channelAuthorization = authorization;
},
}
);
Zotero.debug("WebDAV credentials cached");
this._cachedCredentials = true;
if (this._channelAuthorization) {
Zotero.debug("Authorization header cached");
}
else {
Zotero.debug("No Authorization header to cache");
}
}
catch (e) {
if (e instanceof Zotero.HTTP.UnexpectedStatusException) {
@ -268,7 +284,7 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
Zotero.HTTP.CookieBlocker.removeURL(this._rootURI.spec);
}
this._rootURI = this._parentURI = undefined;
this._cachedCredentials = false;
this._channelAuthorization = false;
},
@ -528,6 +544,7 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
"DELETE",
propURI,
{
headers: this._getAuthorizationHeaders(),
successCodes: [200, 204, 404],
requestObserver: xmlhttp => request.setChannel(xmlhttp.channel),
errorDelayIntervals: this.ERROR_DELAY_INTERVALS,
@ -560,9 +577,12 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
"PUT",
uri,
{
headers: {
"Content-Type": "application/zip"
},
headers: Object.assign(
this._getAuthorizationHeaders(),
{
"Content-Type": "application/zip"
},
),
body: file,
requestObserver: function (req) {
request.setChannel(req.channel);
@ -617,7 +637,6 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
+ "<getcontentlength/>"
+ "</prop></propfind>";
var channel;
var requestObserver = function (req) {
if (options.onRequest) {
options.onRequest(req);
@ -630,13 +649,10 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
uri,
{
successCodes: [200, 204, 404],
requestObserver: function (req) {
if (req.channel) {
channel = req.channel;
}
if (options.onRequest) {
options.onRequest(req);
}
requestObserver,
onAuthorizationHeader: (authorization) => {
// Capture whatever auth it ended up using, if any
this._channelAuthorization = authorization;
},
errorDelayMax: 0,
debug: true
@ -650,20 +666,16 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
throw new this.VerificationError("NOT_DAV", Zotero.HTTP.getDisplayURI(uri, true).spec);
}
var headers = { Depth: 0 };
var contentTypeXML = { "Content-Type": "text/xml; charset=utf-8" };
// Get the Authorization header used in case we need to do a request
// on the parent below
if (channel) {
var channelAuthorization = Zotero.HTTP.getChannelAuthorization(channel);
channel = null;
}
var headers = this._getAuthorizationHeaders();
var propfindHeaders = {
Depth: 0,
"Content-Type": "text/xml; charset=utf-8"
};
// Test whether Zotero directory exists
req = await Zotero.HTTP.request("PROPFIND", uri, {
body: xmlstr,
headers: Object.assign({}, headers, contentTypeXML),
headers: Object.assign({}, headers, propfindHeaders),
successCodes: [207, 404],
requestObserver,
errorDelayMax: 0,
@ -678,6 +690,7 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
"GET",
missingFileURI,
{
headers,
successCodes: [404],
responseType: 'text',
requestObserver,
@ -701,6 +714,7 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
// Test if Zotero directory is writable
let testFileURI = uri.mutate().setSpec(uri.spec + "zotero-test-file.prop").finalize();
req = await Zotero.HTTP.request("PUT", testFileURI, {
headers,
body: " ",
successCodes: [200, 201, 204],
requestObserver,
@ -712,6 +726,7 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
"GET",
testFileURI,
{
headers,
successCodes: [200, 404],
responseType: 'text',
requestObserver,
@ -726,6 +741,7 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
"DELETE",
testFileURI,
{
headers,
successCodes: [200, 204],
requestObserver,
errorDelayMax: 0,
@ -746,16 +762,10 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
}
}
else if (req.status == 404) {
// Include Authorization header from /zotero request,
// since Firefox probably won't apply it to the parent request
if (channelAuthorization) {
headers.Authorization = channelAuthorization;
}
// Zotero directory wasn't found, so see if at least
// the parent directory exists
req = await Zotero.HTTP.request("PROPFIND", parentURI, {
headers: Object.assign({}, headers, contentTypeXML),
headers: Object.assign({}, headers, propfindHeaders),
body: xmlstr,
requestObserver,
successCodes: [207, 404],
@ -1011,6 +1021,8 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
Zotero.debug("Purging orphaned storage files");
await this.cacheCredentials();
var uri = this.rootURI;
var path = uri.pathQueryRef;
@ -1030,7 +1042,11 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
uri,
{
body: xmlstr,
headers: Object.assign({ Depth: 1 }, contentTypeXML),
headers: Object.assign(
this._getAuthorizationHeaders(),
{ Depth: 1 },
contentTypeXML
),
successCodes: [207],
errorDelayIntervals: this.ERROR_DELAY_INTERVALS,
errorDelayMax: this.ERROR_DELAY_MAX,
@ -1158,6 +1174,7 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
"GET",
uri,
{
headers: this._getAuthorizationHeaders(),
successCodes: [200, 300, 404],
responseType: 'text',
requestObserver: xmlhttp => request.setChannel(xmlhttp.channel),
@ -1269,9 +1286,12 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
"PUT",
uri,
{
headers: {
"Content-Type": "text/xml"
},
headers: Object.assign(
{
"Content-Type": "text/xml"
},
this._getAuthorizationHeaders(),
),
body: xmlstr,
successCodes: [200, 201, 204],
errorDelayIntervals: this.ERROR_DELAY_INTERVALS,
@ -1431,6 +1451,7 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
"DELETE",
deleteURI,
{
headers: this._getAuthorizationHeaders(),
successCodes: [200, 204, 404],
errorDelayIntervals: this.ERROR_DELAY_INTERVALS,
errorDelayMax: this.ERROR_DELAY_MAX,
@ -1473,6 +1494,7 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
"DELETE",
deletePropURI,
{
headers: this._getAuthorizationHeaders(),
successCodes: [200, 204, 404],
errorDelayIntervals: this.ERROR_DELAY_INTERVALS,
errorDelayMax: this.ERROR_DELAY_MAX,

296
test/tests/webdavTest.js
View file

@ -9,22 +9,129 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
const davUsername = "user";
const davPassword = "password";
var win, controller, server, requestCount, httpd, davHostPath, davURL;
var responses = {};
var win, controller, httpd, davHostPath, davURL;
var requestCount = 0;
var registeredPaths = new Set();
// Map of path -> { method -> handler }
var pathHandlers = {};
function setResponse(response) {
setHTTPResponse(server, davURL, response, responses, davUsername, davPassword);
/**
* Check Basic Auth credentials from request
*/
function checkAuth(request) {
if (!request.hasHeader('Authorization')) {
return false;
}
let auth = request.getHeader('Authorization');
let expected = 'Basic ' + btoa(davUsername + ':' + davPassword);
return auth == expected;
}
/**
* Send 401 response requiring Basic Auth
*/
function send401(response) {
response.setStatusLine(null, 401, "Unauthorized");
response.setHeader('WWW-Authenticate', 'Basic realm="WebDAV"', false);
}
/**
* Register an httpd handler for a given method/path with optional auth
*/
function setResponse(options) {
let { method, url, status = 200, text = "", headers = {}, handler } = options;
let path = `${davBasePath}${url}`;
// Store the handler info for this method
if (!pathHandlers[path]) {
pathHandlers[path] = {};
}
if (pathHandlers[path][method]) {
throw new Error(`Handler for ${method} ${path} already registered`);
}
pathHandlers[path][method] = { status, text, headers, handler };
// Only register the path handler once -- additional methods on the same path
// reuse the existing handler, which dispatches by method
if (registeredPaths.has(path)) {
return;
}
registeredPaths.add(path);
httpd.registerPathHandler(path, {
handle: function (request, response) {
// Always handle OPTIONS with auth (for cacheCredentials calls)
if (request.method == 'OPTIONS') {
if (!checkAuth(request)) {
send401(response);
return;
}
response.setHeader('DAV', '1', false);
response.setStatusLine(null, 200, "OK");
return;
}
let methodHandlers = pathHandlers[path];
let methodHandler = methodHandlers && methodHandlers[request.method];
if (!methodHandler) {
// No handler for this method -- return 405
response.setStatusLine(null, 405, "Method Not Allowed");
return;
}
// If Authorization not present, send 401 to trigger retry
if (!checkAuth(request)) {
send401(response);
return;
}
requestCount++;
// Custom handler takes precedence
if (methodHandler.handler) {
methodHandler.handler(request, response);
return;
}
// Set status
response.setStatusLine(null, methodHandler.status, null);
// Set headers
for (let [key, value] of Object.entries(methodHandler.headers)) {
response.setHeader(key, String(value), false);
}
// Write body
if (methodHandler.text) {
response.write(methodHandler.text);
}
}
});
}
function resetRequestCount() {
requestCount = server.requests.filter(r => r.responseHeaders["Fake-Server-Match"]).length;
requestCount = 0;
}
function assertRequestCount(count) {
assert.equal(
server.requests.filter(r => r.responseHeaders["Fake-Server-Match"]).length - requestCount,
count
);
assert.equal(requestCount, count);
}
/**
* Unregister all handlers registered via setResponse
*/
function clearRegisteredPaths() {
for (let path of registeredPaths) {
try {
httpd.registerPathHandler(path, null);
}
catch (e) {
// Ignore errors from unregistering paths that weren't registered
}
}
registeredPaths = new Set();
pathHandlers = {};
}
function generateLastSyncID() {
@ -47,10 +154,6 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
skipBundledFiles: true
});
Zotero.HTTP.mock = sinon.FakeXMLHttpRequest;
server = sinon.fakeServer.create();
server.autoRespond = true;
var port;
({ httpd, port } = await startHTTPServer());
davHostPath = `localhost:${port}${davBasePath}`;
@ -82,34 +185,27 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
});
if (!controller.verified) {
setResponse({
method: "OPTIONS",
url: "zotero/",
headers: {
DAV: 1
},
status: 200
})
// Register handlers for server verification
setResponse({
method: "PROPFIND",
url: "zotero/",
status: 207
})
});
setResponse({
method: "PUT",
url: "zotero/zotero-test-file.prop",
status: 201
})
});
setResponse({
method: "GET",
url: "zotero/zotero-test-file.prop",
status: 200
})
});
setResponse({
method: "DELETE",
url: "zotero/zotero-test-file.prop",
status: 200
})
});
await controller.checkServer();
await controller.cacheCredentials();
@ -121,11 +217,11 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
}
afterEach(async function () {
clearRegisteredPaths();
await new Promise(request => httpd.stop(request));
})
after(function* () {
Zotero.HTTP.mock = null;
after(function () {
if (win) {
win.close();
}
@ -213,10 +309,7 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
Zotero.getString('sync.storage.error.webdav.requestError', [500, "GET"])
);
assert.isAbove(
server.requests.filter(r => r.responseHeaders["Fake-Server-Match"]).length - requestCount,
1
);
assert.isAbove(requestCount, 1);
assert.isTrue(library.storageDownloadNeeded);
assert.equal(library.storageVersion, 0);
@ -348,63 +441,86 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
var contentType = 'image/png';
var fileContents = await Zotero.File.getContentsAsync(path);
var deferreds = [];
var zipVerifyDeferred = Zotero.Promise.defer();
setResponse({
method: "GET",
url: `zotero/${item.key}.prop`,
status: 404
});
// https://github.com/cjohansen/Sinon.JS/issues/607
let fixSinonBug = ";charset=utf-8";
server.respond(function (req) {
if (req.username != davUsername) return;
if (req.password != davPassword) return;
if (req.method == "PUT" && req.url == `${davURL}zotero/${item.key}.zip`) {
assert.equal(req.requestHeaders["Content-Type"], "application/zip" + fixSinonBug);
let deferred = Zotero.Promise.defer();
deferreds.push(deferred);
var reader = new FileReader();
reader.addEventListener("loadend", async function () {
try {
let tmpZipPath = OS.Path.join(
Zotero.getTempDirectory().path,
Zotero.Utilities.randomString() + '.zip'
);
let contents = new Uint8Array(reader.result);
await IOUtils.write(tmpZipPath, contents);
// Make sure ZIP file contains the necessary entries
var zr = Components.classes["@mozilla.org/libjar/zip-reader;1"]
.createInstance(Components.interfaces.nsIZipReader);
zr.open(Zotero.File.pathToFile(tmpZipPath));
zr.test(null);
var entries = zr.findEntries('*');
var entryNames = [];
while (entries.hasMore()) {
entryNames.push(entries.getNext());
// Handler for PUT .zip
httpd.registerPathHandler(
`${davBasePath}zotero/${item.key}.zip`,
{
handle: function (request, response) {
if (request.method !== 'PUT') return;
if (!checkAuth(request)) {
send401(response);
return;
}
requestCount++;
// Read request body and verify it's a valid ZIP
let start = async () => {
try {
let bodyStream = request.bodyInputStream;
let bis = Cc["@mozilla.org/binaryinputstream;1"]
.createInstance(Ci.nsIBinaryInputStream);
bis.setInputStream(bodyStream);
let bytes = bis.readByteArray(bis.available());
bis.close();
let tmpZipPath = OS.Path.join(
Zotero.getTempDirectory().path,
Zotero.Utilities.randomString() + '.zip'
);
await IOUtils.write(tmpZipPath, new Uint8Array(bytes));
// Make sure ZIP file contains the necessary entries
var zr = Components.classes["@mozilla.org/libjar/zip-reader;1"]
.createInstance(Components.interfaces.nsIZipReader);
zr.open(Zotero.File.pathToFile(tmpZipPath));
zr.test(null);
var entries = zr.findEntries('*');
var entryNames = [];
while (entries.hasMore()) {
entryNames.push(entries.getNext());
}
assert.equal(entryNames.length, 1);
assert.sameMembers(entryNames, [filename]);
assert.equal(zr.getEntry(filename).realSize, size);
await OS.File.remove(tmpZipPath);
zipVerifyDeferred.resolve();
}
assert.equal(entryNames.length, 1);
assert.sameMembers(entryNames, [filename]);
assert.equal(zr.getEntry(filename).realSize, size);
await OS.File.remove(tmpZipPath);
deferred.resolve();
}
catch (e) {
deferred.reject(e);
}
});
reader.readAsArrayBuffer(req.requestBody);
req.respond(201, { "Fake-Server-Match": 1 }, "");
catch (e) {
zipVerifyDeferred.reject(e);
}
};
start();
response.setStatusLine(null, 201, null);
}
}
else if (req.method == "PUT" && req.url == `${davURL}zotero/${item.key}.prop`) {
);
// Handler for PUT .prop
setResponse({
method: "PUT",
url: `zotero/${item.key}.prop`,
handler: function (request, response) {
// Read and verify request body
let bodyStream = request.bodyInputStream;
let sis = Cc["@mozilla.org/scriptableinputstream;1"]
.createInstance(Ci.nsIScriptableInputStream);
sis.init(bodyStream);
let body = sis.read(sis.available());
sis.close();
var parser = new DOMParser();
var doc = parser.parseFromString(req.requestBody, "text/xml");
var doc = parser.parseFromString(body, "text/xml");
assert.equal(
doc.documentElement.getElementsByTagName('mtime')[0].textContent, mtime
);
@ -412,13 +528,13 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
doc.documentElement.getElementsByTagName('hash')[0].textContent, hash
);
req.respond(204, { "Fake-Server-Match": 1 }, "");
response.setStatusLine(null, 204, null);
}
});
var result = await engine.start();
await Promise.all(deferreds.map(d => d.promise));
await zipVerifyDeferred.promise;
assertRequestCount(3);
@ -579,8 +695,7 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
//
// https://forums.zotero.org/discussion/80429/sync-error-in-5-0-80
it("shouldn't send cookies", async function () {
// Make real requests so we can test the internal cookie-handling behavior
Zotero.HTTP.mock = null;
// Skip initial verification for this test
controller.verified = true;
var engine = await setup();
@ -645,12 +760,9 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
response.setStatusLine(null, 400, "Bad Request");
return;
}
// Authorization used to already be cached here, but that's no longer the
// case as of fx128, so send 401
// Should already include Authorization
if (!request.hasHeader('Authorization')) {
//response.setStatusLine(null, 400, "");
response.setStatusLine(null, 401, null);
response.setHeader('WWW-Authenticate', 'Basic realm="WebDAV"', false);
response.setStatusLine(null, 400, "");
return;
}
// Cookie shouldn't be passed
@ -747,7 +859,6 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
describe("Verify Server", function () {
it("should show an error for a connection error", async function () {
Zotero.HTTP.mock = null;
Zotero.Prefs.set("sync.storage.url", "127.0.0.1:9999");
// Begin install procedure
@ -772,7 +883,6 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
});
it("should show an error for a non-DAV URL", async function () {
Zotero.HTTP.mock = null;
Zotero.Prefs.set("sync.storage.url", davHostPath);
httpd.registerPathHandler(
@ -816,7 +926,6 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
});
it("should show an error for a 403", async function () {
Zotero.HTTP.mock = null;
httpd.registerPathHandler(
`${davBasePath}zotero/`,
{
@ -826,7 +935,6 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
}
);
// Use httpd.js instead of sinon so we get a real nsIURL with a channel
Zotero.Prefs.set("sync.storage.url", davHostPath);
// Begin install procedure
@ -852,8 +960,6 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
it("should show an error for a 404 for the parent directory", async function () {
// Use httpd.js instead of sinon so we get a real nsIURL with a channel
Zotero.HTTP.mock = null;
Zotero.Prefs.set("sync.storage.url", davHostPath);
httpd.registerPathHandler(
@ -909,7 +1015,6 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
it("should show an error for a 200 for a nonexistent file", async function () {
Zotero.HTTP.mock = null;
httpd.registerPathHandler(
`${davBasePath}zotero/`,
{
@ -940,7 +1045,6 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
}
);
// Use httpd.js instead of sinon so we get a real nsIURL with a channel
Zotero.Prefs.set("sync.storage.url", davHostPath);
// Begin install procedure