Skip to content

Commit 36866cb

Browse files
committed
fix by Copilot's review
1 parent ee4429b commit 36866cb

2 files changed

Lines changed: 25 additions & 15 deletions

File tree

packages/devextreme/js/__internal/core/utils/m_data.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,14 @@ export const compileGetter = function (expr) {
6868

6969
if (typeof expr === 'string') {
7070
const path = getPathParts(expr);
71+
const unsafeFragment = path.find(isUnsafePathFragment);
7172

7273
return function (obj, options) {
74+
if (unsafeFragment !== undefined) {
75+
errors.log('E0123', unsafeFragment);
76+
return;
77+
}
78+
7379
options = prepareOptions(options);
7480
const functionAsIs = options.functionsAsIs;
7581
const hasDefaultValue = 'defaultValue' in options;
@@ -85,11 +91,6 @@ export const compileGetter = function (expr) {
8591

8692
const pathPart = path[i];
8793

88-
if (isUnsafePathFragment(pathPart)) {
89-
errors.log('E0123', pathPart);
90-
return;
91-
}
92-
9394
if (hasDefaultValue && isObject(current) && !(pathPart in current)) {
9495
return options.defaultValue;
9596
}
@@ -173,11 +174,11 @@ const ensurePropValueDefined = function (obj, propName, value, options) {
173174
export const compileSetter = function (expr) {
174175
expr = getPathParts(expr || 'this');
175176
const lastLevelIndex = expr.length - 1;
177+
const unsafeFragment = expr.find(isUnsafePathFragment);
176178

177179
return function (obj, value, options) {
178180
options = prepareOptions(options);
179181

180-
const unsafeFragment = expr.find(isUnsafePathFragment);
181182
if (unsafeFragment !== undefined) {
182183
errors.log('E0123', unsafeFragment);
183184
return;

packages/devextreme/testing/tests/DevExpress.core/utils.data.tests.js

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ QUnit.module('prototype pollution protection', {
492492
}
493493
}, () => {
494494

495-
test('compileSetter logs error and skips assignment for __proto__ path fragment', function(assert) {
495+
test('compileSetter logs error and skips assignment for __proto__ path fragment (T1330839)', function(assert) {
496496
const obj = {};
497497
SETTER('__proto__.pp_dx')(obj, 'yes', { functionsAsIs: true });
498498

@@ -501,7 +501,7 @@ QUnit.module('prototype pollution protection', {
501501
assert.strictEqual(({}).pp_dx, undefined, 'Object.prototype must not be polluted');
502502
});
503503

504-
test('compileSetter logs error and skips assignment for constructor path fragment', function(assert) {
504+
test('compileSetter logs error and skips assignment for constructor path fragment (T1330839)', function(assert) {
505505
const obj = {};
506506
SETTER('constructor.prototype.pp_dx')(obj, 'yes', { functionsAsIs: true });
507507

@@ -510,7 +510,7 @@ QUnit.module('prototype pollution protection', {
510510
assert.strictEqual(({}).pp_dx, undefined, 'Object.prototype must not be polluted');
511511
});
512512

513-
test('compileSetter logs error and skips assignment for prototype path fragment', function(assert) {
513+
test('compileSetter logs error and skips assignment for prototype path fragment (T1330839)', function(assert) {
514514
const fn = function() {};
515515
SETTER('prototype.pp_dx')(fn, 'yes', { functionsAsIs: true });
516516

@@ -519,36 +519,36 @@ QUnit.module('prototype pollution protection', {
519519
assert.strictEqual(fn.prototype.pp_dx, undefined, 'function prototype must not be modified');
520520
});
521521

522-
test('compileSetter works normally for safe paths', function(assert) {
522+
test('compileSetter works normally for safe paths (T1330839)', function(assert) {
523523
const obj = { a: { b: 1 } };
524524
SETTER('a.b')(obj, 42);
525525

526526
assert.strictEqual(obj.a.b, 42, 'safe paths must still work');
527527
assert.strictEqual(errors.log.called, false, 'should not log for safe paths');
528528
});
529529

530-
test('compileGetter logs error and returns undefined for __proto__ path fragment', function(assert) {
530+
test('compileGetter logs error and returns undefined for __proto__ path fragment (T1330839)', function(assert) {
531531
const result = GETTER('__proto__.pp_dx')({});
532532

533533
assert.strictEqual(errors.log.calledWith('E0123', '__proto__'), true, 'should log E0123 for __proto__');
534534
assert.strictEqual(result, undefined, 'getter must return undefined for __proto__');
535535
});
536536

537-
test('compileGetter logs error and returns undefined for constructor path fragment', function(assert) {
537+
test('compileGetter logs error and returns undefined for constructor path fragment (T1330839)', function(assert) {
538538
const result = GETTER('constructor.prototype')(function() {});
539539

540540
assert.strictEqual(errors.log.calledWith('E0123', 'constructor'), true, 'should log E0123 for constructor');
541541
assert.strictEqual(result, undefined, 'getter must return undefined for constructor');
542542
});
543543

544-
test('compileGetter logs error and returns undefined for prototype path fragment', function(assert) {
544+
test('compileGetter logs error and returns undefined for prototype path fragment (T1330839)', function(assert) {
545545
const result = GETTER('prototype.pp_dx')(function() {});
546546

547547
assert.strictEqual(errors.log.calledWith('E0123', 'prototype'), true, 'should log E0123 for prototype');
548548
assert.strictEqual(result, undefined, 'getter must return undefined for prototype');
549549
});
550550

551-
test('combineGetters logs error and skips __proto__ fragment, returns safe fields', function(assert) {
551+
test('combineGetters logs error and skips __proto__ fragment, returns safe fields (T1330839)', function(assert) {
552552
const obj = { safe: 'value' };
553553
const result = GETTER(['__proto__.pp_dx', 'safe'])(obj);
554554

@@ -557,14 +557,23 @@ QUnit.module('prototype pollution protection', {
557557
assert.deepEqual(result, { safe: 'value' }, 'safe field must still be returned');
558558
});
559559

560-
test('combineGetters logs error and skips constructor fragment, returns safe fields', function(assert) {
560+
test('combineGetters logs error and skips constructor fragment, returns safe fields (T1330839)', function(assert) {
561561
const obj = { safe: 'value' };
562562
const result = GETTER(['constructor.prototype.pp_dx', 'safe'])(obj);
563563

564564
assert.strictEqual(errors.log.calledWith('E0123', 'constructor'), true, 'should log E0123 for constructor');
565565
assert.strictEqual(({}).pp_dx, undefined, 'Object.prototype must not be polluted');
566566
assert.deepEqual(result, { safe: 'value' }, 'safe field must still be returned');
567567
});
568+
569+
test('combineGetters logs error and skips unsafe paths even if intermediate values break early (with defaultValue) (T1330839)', function(assert) {
570+
const obj = { a: null, safe: 'value' };
571+
const result = GETTER(['a.constructor.prototype.pp_dx2', 'safe'])(obj, { defaultValue: 'default' });
572+
573+
assert.strictEqual(errors.log.calledWith('E0123', 'constructor'), true, 'should log E0123 for constructor');
574+
assert.strictEqual(({}).pp_dx2, undefined, 'Object.prototype must not be polluted');
575+
assert.deepEqual(result, { safe: 'value' }, 'safe field must still be returned');
576+
});
568577
});
569578

570579

0 commit comments

Comments
 (0)