Skip to content

Commit

Permalink
fix: non-fk access in filter conditions are properly rejected (#336)
Browse files Browse the repository at this point in the history
  • Loading branch information
patricebender authored Nov 10, 2023
1 parent 26c6650 commit 4c948fe
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 17 deletions.
40 changes: 26 additions & 14 deletions db-service/lib/infer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -499,20 +499,7 @@ function infer(originalQuery, model = cds.context?.model || cds.model) {
const elements = definition.elements || definition._target?.elements
if (elements && id in elements) {
const element = elements[id]
if (!inNestedProjection && !inCalcElement && element.target) {
// only fk access in infix filter
const nextStep = column.ref[1]?.id || column.ref[1]
// no unmanaged assoc in infix filter path
if (!inExists && element.on)
throw new Error(
`"${element.name}" in path "${column.ref
.map(idOnly)
.join('.')}" must not be an unmanaged association`,
)
// no non-fk traversal in infix filter
if (nextStep && element.foreignKeys && !(nextStep in element.foreignKeys))
throw new Error(`Only foreign keys of "${element.name}" can be accessed in infix filter`)
}
rejectNonFkAccess(element)
const resolvableIn = definition.target ? definition._target : target
column.$refLinks.push({ definition: elements[id], target: resolvableIn })
} else {
Expand Down Expand Up @@ -557,6 +544,8 @@ function infer(originalQuery, model = cds.context?.model || cds.model) {

const target = definition._target || column.$refLinks[i - 1].target
if (element) {
if($baseLink)
rejectNonFkAccess(element)
const $refLink = { definition: elements[id], target }
column.$refLinks.push($refLink)
} else if (firstStepIsSelf) {
Expand Down Expand Up @@ -658,6 +647,29 @@ function infer(originalQuery, model = cds.context?.model || cds.model) {
}
}
}

/**
* Check if the next step in the ref is foreign key of `element`
* if not, an error is thrown.
*
* @param {CSN.Element} element if this is an association, the next step must be a foreign key of the element.
*/
function rejectNonFkAccess(element) {
if (!inNestedProjection && !inCalcElement && element.target) {
// only fk access in infix filter
const nextStep = column.ref[i + 1]?.id || column.ref[i + 1]
// no unmanaged assoc in infix filter path
if (!inExists && element.on)
throw new Error(
`"${element.name}" in path "${column.ref
.map(idOnly)
.join('.')}" must not be an unmanaged association`
)
// no non-fk traversal in infix filter
if (nextStep && element.foreignKeys && !(nextStep in element.foreignKeys))
throw new Error(`Only foreign keys of "${element.name}" can be accessed in infix filter`)
}
}
})

// ignore whole expand if target of assoc along path has ”@cds.persistence.skip”
Expand Down
14 changes: 11 additions & 3 deletions db-service/test/cqn4sql/where-exists.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,13 +209,21 @@ describe('EXISTS predicate in where', () => {
)`)
})

it.skip('MUST fail if following managed assoc in filter', () => {
it('MUST fail if following managed assoc in filter in where exists', () => {
expect(() =>
cqn4sql(
CQL`SELECT from bookshop.Authors { ID } WHERE EXISTS books[dedication.addressee.name = 'Hasso']`,
model,
),
).to.throw()
).to.throw('Only foreign keys of "addressee" can be accessed in infix filter')
})
it('MUST fail if following managed assoc in filter', () => {
expect(() =>
cqn4sql(
CQL`SELECT from bookshop.Authors { ID, books[dedication.addressee.name = 'Hasso'].dedication.addressee.name as Hasso }`,
model,
),
).to.throw('Only foreign keys of "addressee" can be accessed in infix filter')
})

it('MUST handle simple where exists with multiple association and also with $self backlink', () => {
Expand All @@ -230,7 +238,7 @@ describe('EXISTS predicate in where', () => {
)`)
})

it('MUST handle simple where exists with additional filter, shourcut notation', () => {
it('MUST handle simple where exists with additional filter, shortcut notation', () => {
let query = cqn4sql(CQL`SELECT from bookshop.Books { ID } where exists author[17]`, model)
expect(query).to.deep.equal(CQL`SELECT from bookshop.Books as Books { Books.ID } WHERE EXISTS (
SELECT 1 from bookshop.Authors as author where author.ID = Books.author_ID and author.ID = 17
Expand Down

0 comments on commit 4c948fe

Please sign in to comment.