setAjaxHeaders - headers do not get updated to the latest value

[danaLumy]

I've tried to use the setAjaxHeaders method to update the Authorization header when the token expires but the header for TiledImage and Tiles is still the expired token. I've read the implementation of the setAjaxHeaders and it seems that the merging of the headers is done incorrect in tiledimage.js line 1059 and line 1081. The way the parameters are ordered in the extend() method will result in having the old values as a result of the merging. Is this behaviour intended or is this a bug?

[iangilman]

Good question! That function was added with #2346. @uschmidt83 any thoughts?

[uschmidt83]

Hi @danaLumy, can you please provide a minimal reproducible example that shows the issue?

Maybe it's also helpful to look at the existing header-related tests that show the expected behavior.

[danaLumy]

Hi @uschmidt83 I have a viewer with multiple tiledImage configured like this:

customViewer.addTiledImage({
        tileSource: {
          height: this.image.height,
          width: this.image.width,
          tileSize: this.config.serverSettings?.tileSize,
          getTileUrl: function (zoomLevel: number, xCoordinate: number, yCoordinate: number) {
            return imageService.getChannelTiledImage(imageName, zoomLevel, xCoordinate, yCoordinate, channel.name);
          }
        },
        loadTilesWithAjax: true,
        ajaxHeaders: {
          "Authorization": 'Bearer ${this.token}'
        },
        crossOriginPolicy: 'Anonymous',
        compositeOperation: "lighter"
      });

In a method I call viewer.setAjaxHeaders({ "Authorization": 'Bearer Nothing'},true); and I log the header for each TiledImage and I get the header with a valid token.
I've tried to call viewer.world.getItemAt(i).setAjaxHeaders({ "Authorization": 'Bearer Nothing'},true); and this works as expected, the header is updated to Bearer Nothing.
I've tried to read the code in the tiledimage.js and as I understood the code this is the first init of the ajaxHeaders:

this._ownAjaxHeaders = {};
this.setAjaxHeaders(ajaxHeaders, false);

and in the _updateAjaxHeaders the headers from the viewer ("Authorization": 'Bearer Nothing') are merged with the this._ownAjaxHeaders (("Authorization": 'Bearer Valid Token') resulting in a header with the value of the tiledImage and not the viewer. Please let me know if this is an intended behaviour.

[pearcetm]

I agree with @danaLumy - it looks like the order of arguments in $.extend is wrong, and causes the TiledImage header values to overwrite those of the Viewer rather than the other way around:

openseadragon/src/tiledimage.js

Line 1059 in 5be1ab6

this.ajaxHeaders = $.extend({}, this.viewer.ajaxHeaders, this._ownAjaxHeaders);

[danaLumy]

@pearcetm I think I was wrong in my first assumption that the order of the params is wrong, the viewer uses the __updateAjaxHeaders from tiledimage.js, where this._ownAjaxHeaders is never set with the new value, but if you use the setAjaxHeaders from tileImaged in viewer at line 1013 then I think it will work as expected, but I don't know the reason for using the __updateAjaxHeaders at line 1013, based on the documentation I was not able to understand why so I'll just wait to see what @uschmidt83 thinks of all this.

[pearcetm]

It isn't necessarily a bug, because the behavior you're seeing is described in the documentation (emphasis mine):

Update headers to include when making AJAX requests. Unless propagate is set to false (which is likely only useful in rare circumstances), the updated headers are propagated to all tiled images, each of which will subsequently propagate the changed headers to all their tiles. If applicable, the headers of the viewer's navigator and reference strip will also be updated. Note that the rules for merging headers still apply, i.e. headers returned by OpenSeadragon.TileSource#getTileAjaxHeaders take precedence over TiledImage.ajaxHeaders, which take precedence over the headers here in the viewer.

It is a bit counterintuitive, however, that propagating changes to the TiledImages and then to Tiles doesn't actually override the current values. You'd have to manually loop over all of these things to update their existing values. In my opinion it would be useful to have a supported API for making this type of update.

[uschmidt83]

Hi @danaLumy,

it also took me a while to re-familarize myself with my own code.
That probably means the documentation should be improved :)

Please let me know if this is an intended behaviour.

Yes, it is.

@pearcetm quoted the relevant part from the documentation that explains the order in which headers take precedence (Tile > TiledImage > Viewer).

The idea is that you put your most generic headers in the Viewer, source-specific headers in the corresponding TiledImage and tile-specific headers into the TileSource.getTileAjaxHeaders(level, x, y) function, which will be called when a Tile is created.

In your example code, you put an Authorization header into a specific TiledImage, but then try to override by calling the Viewer.setAjaxHeaders function. And this doesn't work because the Authorization header already exists in the TiledImage and thus takes precedence over the new value to you put in the Viewer.

If your Authorization header is the same for all TiledImages, you can set (and update) it only in the Viewer and it should work as expected.

---

Hi @pearcetm,

You'd have to manually loop over all of these things to update their existing values. In my opinion it would be useful to have a supported API for making this type of update.

can you explain to me what use case you're thinking of and why the current implementation doesn't work for it?

[pearcetm]

can you explain to me what use case you're thinking of and why the current implementation doesn't work for it?

@uschmidt83 I think the current implementation is very solid and for most cases does exactly the right thing. Perhaps adding a third parameter to the function signature: setAjaxHeaders(headers, propagate, overrideChildHeaders) (or something) could provide a convenient way to update parameters top-down. @danaLumy's use case is an example - an AJAX parameter was initially explicitly set at the TiledImage level, but the dev now wants to be able to override that for all of the existing TiledImages. While uncommon, I could see this being useful if an initial generic credential was used at page load, and if/when a user logs in, their personal credentials should be used instead. Rather than having to iterate over and update all TiledImages individually, the viewer could provide a convenience function for this.

I'm not saying this should be done - it may not be a good idea at all. One can make an argument that devs should update all TiledImages explicitly themselves in such a scenario if needed. If a convenience function/convenience parameter for such an override isn't added, maybe a bit more explicit documentation would help.

[iangilman]

Yeah, I'd say improving the documentation is the first step here.

[pearcetm]

I think the reason I find the current API a bit confusing is the propagate parameter. With propagate == true all existing things with ajaxHeaders are refreshed to take account of viewer's new headers (and, as has been discussed, they will still override the viewers new headers with their own). To me, a more common use of "propagate" is to actually apply said change to all children, which in our case would be equivalent to (pseudocode) viewer.world.tiledImages.forEach(t => t.setAjaxHeaders(newHeaders) ).

The other part of it is, I'm struggling to think of a scenario in which propagate == false would make sense. You would essentially need to want to update the viewer's ajax headers only for newly added tiled images. You'd end up in a situation where different tiled images inherited different headers from the viewer, based on when they were opened. Since navigation can trigger new tile requests, some tiles would therefore be requested with outdated viewer headers.

[iangilman]

@pearcetm Good points! @uschmidt83 What are your thoughts?

[uschmidt83]

Sorry for being silent for a while.

If I remember correctly, OpenSeadragon already had the concept of AJAX headers that either belong to either the viewer, a tiled image, or a tile, and the rules how to merge them. I simply wrote some code to update the headers while respecting the concept that was already in place.

@danaLumy's use case is an example - an AJAX parameter was initially explicitly set at the TiledImage level, but the dev now wants to be able to override that for all of the existing TiledImages. While uncommon, I could see this being useful if an initial generic credential was used at page load, and if/when a user logs in, their personal credentials should be used instead. Rather than having to iterate over and update all TiledImages individually, the viewer could provide a convenience function for this.

It seems to me that this scenario would still be better served by just setting the headers for the viewer. I think the problem is rather documentation to tell users that headers should always be set for the viewer, unless different headers are needed for each tiled image.

The other part of it is, I'm struggling to think of a scenario in which propagate == false would make sense. You would essentially need to want to update the viewer's ajax headers only for newly added tiled images. You'd end up in a situation where different tiled images inherited different headers from the viewer, based on when they were opened. Since navigation can trigger new tile requests, some tiles would therefore be requested with outdated viewer headers.

I agree, that's why I also mentioned it in the documentation:

openseadragon/src/viewer.js

Lines 1043 to 1048 in 59645e3

	/**
	* Update headers to include when making AJAX requests.
	*
	* Unless `propagate` is set to false (which is likely only useful in rare circumstances),
	* the updated headers are propagated to all tiled images, each of which will subsequently
	* propagate the changed headers to all their tiles.

To me, a more common use of "propagate" is to actually apply said change to all children, which in our case would be equivalent to (pseudocode) viewer.world.tiledImages.forEach(t => t.setAjaxHeaders(newHeaders) ).

To be honest, I personally don't really care if the API is modified or amended. (For various reasons, I never actually got to use the code that I contributed in #2346.) But I agree that the documentation should be improved to be more explicit about common uses cases and pitfalls.

[danaLumy]

It seems to me that this scenario would still be better served by just setting the headers for the viewer. I think the problem is rather documentation to tell users that headers should always be set for the viewer, unless different headers are needed for each tiled image.

@uschmidt83 I think that when integrating OSD we've missed this aspect, and now I'm reconsidering the way we set the headers, because we need all the time the headers set on the viewer. Thank you for taking the time to clear for me the implementation and also the order of the headers. @iangilman, @pearcetm I agree that improving the documentation would make it easier to configure the headers. Thank you, this answered my question.

[iangilman]

Thank you all! I've marked this as a documentation issue... It's open for anyone who wants to take a whack at improving the docs about it :)

[ghost]

hey @iangilman , could you please assign this issue to me so that i can work on it.

[iangilman]

@hem4nn Great! Do you understand what needs documenting here? I realize the conversation has had a lot of back and forth...