Skip to content

Commit

Permalink
[FIX] v2.ODataModel: Set "X-Requested-With" header
Browse files Browse the repository at this point in the history
The "X-Requested-With" being set to "XMLHttpRequest" for
OData requests properly indicate that these requests are
used to fetch data for non-crossOrigin requests.
Cross origin requests can cause unwanted preflight requests
with this header being set.
It is also not a CORS-safelist request-header.

This behaviour is in sync with jQuery.ajax which is used
by OData V4.

PS1: initial implementation with tests
PS2: fix unit test
PS3: review comments
PS4: review comments
PS5: review comments
PS6: review comments
PS7: review comments
PS8: review comments
PS9: review comments
PS10: review comments

BCP: 2170172149
Closes: #3324
Change-Id: I1fb8bd23b9a65fb8361ddbad0d170ccf49b40771
  • Loading branch information
tobiasso85 committed Sep 7, 2021
1 parent 4c9905c commit 15acac7
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 6 deletions.
12 changes: 10 additions & 2 deletions src/sap.ui.core/src/sap/ui/model/odata/v2/ODataModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,14 @@ sap.ui.define([
"sap/ui/model/odata/OperationMode",
"sap/ui/model/odata/UpdateMethod",
"sap/ui/thirdparty/datajs",
"sap/ui/thirdparty/URI"
"sap/ui/thirdparty/URI",
"sap/ui/util/isCrossOriginURL"
], function(Context, ODataAnnotations, ODataContextBinding, ODataListBinding, ODataTreeBinding,
assert, Log, encodeURL, deepEqual, deepExtend, each, extend, isEmptyObject, isPlainObject,
merge, uid, UriParameters, coreLibrary, Message, MessageParser, BindingMode, BaseContext,
FilterProcessor, Model, CountMode, MessageScope, ODataMetadata, ODataMetaModel,
ODataMessageParser, ODataPropertyBinding, ODataUtils, OperationMode, UpdateMethod, OData,
URI
URI, isCrossOriginURL
) {

"use strict";
Expand Down Expand Up @@ -485,6 +486,13 @@ sap.ui.define([
this.oHeaders["MaxDataServiceVersion"] = this.sMaxDataServiceVersion;
}

// "XMLHttpRequest" indicates that a data request is performed.
// Gets only applied to non-crossOrigin requests because cross origin requests could
// result in unwanted preflight requests if this header is set.
// This behaviour is in sync with jQuery.ajax which is used by OData V4.
if (!this.mCustomHeaders["X-Requested-With"] && !isCrossOriginURL(this.sServiceUrl)) {
this.oHeaders["X-Requested-With"] = "XMLHttpRequest";
}
},
metadata : {
publicMethods : ["read", "create", "update", "remove", "submitChanges", "getServiceMetadata", "metadataLoaded",
Expand Down
2 changes: 1 addition & 1 deletion src/sap.ui.core/src/sap/ui/util/isCrossOriginURL.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ sap.ui.define([], function() {
* @param {sap.ui.core.URI} sHref The URL to check
* @returns {boolean} Whether the URL is a cross-origin URL
* @private
* @ui5-restricted
* @ui5-restricted sap.ui.model.odata.v2.ODataModel,sap.ushell
* @alias module:sap/ui/util/isCrossOriginURL
* @since 1.84
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,7 @@ sap.ui.define([
delete mHeaders["Content-Type"];
delete mHeaders["DataServiceVersion"];
delete mHeaders["MaxDataServiceVersion"];
delete mHeaders["X-Requested-With"];
delete mHeaders["sap-cancel-on-close"];
delete mHeaders["sap-contextid-accept"];
delete oActualRequest["_handle"];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,33 @@ sap.ui.define([
});

//*********************************************************************************************
QUnit.test("constructor: codeListModelParameters and sMetadataUrl stored", function (assert) {
[{
sExpectedRequestedWithHeader : "XMLHttpRequest",
sServiceUrl : "/foo/bar"
}, {
sServiceUrl : "/foo/bar",
oHeaderParameter : {
"X-Requested-With" : "~X-Requested-With"
}
}, {
sServiceUrl : "https://example.com/foo/bar"
}, {
oHeaderParameter : {
"X-Requested-With" : "~X-Requested-With"
},
sServiceUrl : "https://example.com/foo/bar"
}].forEach(function (oFixture, i) {
var sTitle = "constructor: codeListModelParameters and sMetadataUrl stored #" + i
+ ", sServiceUrl: " + oFixture.sServiceUrl;
QUnit.test(sTitle, function (assert) {
var oDataModelMock = this.mock(ODataModel),
oExpectedHeaders = {
"Accept" : "application/json",
"Accept-Language" : "~languageTag",
"DataServiceVersion" : "2.0",
"MaxDataServiceVersion" : "2.0",
"sap-contextid-accept" : "header"
},
oMetadata = {
oMetadata : {
isLoaded : function () {},
Expand All @@ -72,7 +97,8 @@ sap.ui.define([
},
mParameters = {
annotationURI : "~annotationURI",
serviceUrl : "/foo/bar",
headers : oFixture.oHeaderParameter || {},
serviceUrl : oFixture.sServiceUrl,
skipMetadataAnnotationParsing : true,
tokenHandling : false
},
Expand All @@ -90,7 +116,7 @@ sap.ui.define([
.returns("~serverUrl");
oDataModelMock.expects("_getSharedData").withExactArgs("server", "~serverUrl")
.returns(undefined);
oDataModelMock.expects("_getSharedData").withExactArgs("service", "/foo/bar")
oDataModelMock.expects("_getSharedData").withExactArgs("service", oFixture.sServiceUrl)
.returns(undefined);
oDataModelMock.expects("_getSharedData").withExactArgs("meta", "~metadataUrl")
.returns(oMetadata);
Expand All @@ -102,13 +128,21 @@ sap.ui.define([
this.mock(oMetadata.oMetadata).expects("isLoaded").withExactArgs().returns(true);
this.mock(ODataAnnotations.prototype).expects("addSource")
.withExactArgs(["~annotationURI"]);
this.mock(sap.ui.getCore().getConfiguration()).expects("getLanguageTag").withExactArgs()
.returns("~languageTag");
if (oFixture.sExpectedRequestedWithHeader) {
oExpectedHeaders["X-Requested-With"] = oFixture.sExpectedRequestedWithHeader;
}

// code under test
var oModel = new ODataModel(mParameters);

assert.strictEqual(oModel.mCodeListModelParams, "~codeListModelParameters");
assert.strictEqual(oModel.sMetadataUrl, "~metadataUrl");
assert.deepEqual(oModel.oHeaders, oExpectedHeaders);
assert.deepEqual(oModel.mCustomHeaders, oFixture.oHeaderParameter || {});
});
});

//*********************************************************************************************
QUnit.test("read: updateAggregatedMessages passed to _createRequest", function (assert) {
Expand Down

0 comments on commit 15acac7

Please sign in to comment.