添加链接
link管理
链接快照平台
  • 输入网页链接,自动生成快照
  • 标签化管理网页链接

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement . We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug description

Calling findFirst with an undefined in where filter still returns data. This is not intuitive.

How to reproduce

  • Deploy this schema with yarn prisma db push --preview-feature
  • datasource ds {
        provider = "sqlite"
        url      = "file:./dev.db"
    generator pc {
        provider = "prisma-client-js"
    model User {
        id   String @id
        name String
    
  • Ensure it has some rows
  • Run the following script
  • const { PrismaClient } = require('@prisma/client')
    const prisma = new PrismaClient()
    async function main() {
      const data = await prisma.user.findFirst({
        where: {
          id: null
      console.log({ data })
    main()
  • It returns
  • { data: { id: '1', name: 'Alice' } }
    
  • The where ID doesn't match anything, it shouldn't return
  • Expected behaviour

    IMO, this should be a runtime error.

    Prisma information

    (base) divyendusingh [p2-ff]$ yarn prisma --version                                                                                                                                  130 ↵
    yarn run v1.22.4
    $ /Users/divyendusingh/Documents/prisma/triage/p2-ff/node_modules/.bin/prisma --version
    @prisma/cli          : 2.15.0-dev.57
    @prisma/client       : 2.15.0-dev.57
    Current platform     : darwin
    Query Engine         : query-engine 6cece221de1da7dd83b9a158db5147e36f802cb1 (at node_modules/@prisma/engines/query-engine-darwin)
    Migration Engine     : migration-engine-cli 6cece221de1da7dd83b9a158db5147e36f802cb1 (at node_modules/@prisma/engines/migration-engine-darwin)
    Introspection Engine : introspection-core 6cece221de1da7dd83b9a158db5147e36f802cb1 (at node_modules/@prisma/engines/introspection-engine-darwin)
    Format Binary        : prisma-fmt 6cece221de1da7dd83b9a158db5147e36f802cb1 (at node_modules/@prisma/engines/prisma-fmt-darwin)
    Studio               : 0.333.0
              

    This would be good to include in big red text in Sequelize migration tips&tricks. We have just released Prisma to production, where we replaced thousands of lines of code from Sequelize. All in all Prisma is awesome, but this one thing introduced nasty bug. Of course due to our fault, but as the issue mentioned, this is not intuitive.

    Can there be a flag where you can control the behaviour? The case is that it happens a lot in auth code etc where you do

       const device = await prisma.deviceModel.findFirst({ where: { deviceId } });
    

    If by accident you pass undefined here, the random model is returned which is really dangerous in some cases.

    Have just come across this and I was pretty surprised by it.

    I would also describe it as dangerous, mostly because I'd guess that most people would not assume this behaviour, and will instead rely on error handling code after the query to check if nothing was found when an undefined id is passed, for example.

    I think it would be great to introduce as a feature the ability to configure this behaviour off globally.

    I can see an argument that typescript can eliminate this bug too. The reason I came across it was admittedly due to a lazy workaround on my part -- I was asserting a query param was present in TS with the !, then the query param name changed, resulting in it being undefined. Really I should have used a zod-like runtime type guard. But, even in the case of using TS, examples like this of things not being quite perfect and undefineds sneaking in at runtime abound. I really think it's better to have the additional failsafe here.

    This has the be one of the worse design decisions. We also just found a bug where findFirst returns another user’s record. The problem is it does this silently.

    And not just query, but it could happen in update/delete as well. Really dangerous.

    For anyone who is currently struggling with this issue. The following workaround could use prisma middleware to prevent any undefined appears in where. For me, this is handy that I could prevent to make mistakes which will let the undefined to sneaked in.

    Write a custom prismaClient instance which include the middleware. The middleware will check the params.args.where and throw an Error if any undefined has been included.

    import { PrismaClient } from '.prisma/client';
    const prisma = new PrismaClient();
    prisma.$use(async (params, next) => {
      if (hasUndefinedValue(params.args?.where))
        throw new Error(
          `Invalid where: ${JSON.stringify(params.args.where)}`
      return await next(params);
    });
    export default prisma;
    function hasUndefinedValue<T>(obj: T): boolean {
      if (typeof obj !== 'object' || obj === null) return false;
      for (const key in obj) {
        if (!Object.prototype.hasOwnProperty.call(obj, key)) continue;
        const value = obj[key];
        if (value === undefined) return true;
        if (typeof value === 'object' && !Array.isArray(value))
          if (hasUndefinedValue(value)) return true;
      return false;
    

    Then you could export and use the prismaClient instance as usual. It will also be better to have the test case to verify the middleware is working fine:

    describe('with undefined', () => {
      it('should throw Error', async () => {
        const error = new Error('Invalid where: {}');
        await expect(
          prisma.user.findUnique({ where: { email: undefined } })
        ).rejects.toThrow(error);
      });
    });

    Hope this help somebody!

    This is sorta weird Prisma behavior, if the main attribute of the where
    clause is `undefined` in a `findFirst` query, it will exclude that
    condition and then it is just going to grab the first row from the table
    and return that. prisma/prisma#5149
    This is sorta weird Prisma behavior, if the main attribute of the where
    clause is `undefined` in a `findFirst` query, it will exclude that
    condition and then it is just going to grab the first row from the table
    and return that. prisma/prisma#5149
    This is sorta weird Prisma behavior, if the main attribute of the where
    clause is `undefined` in a `findFirst` query, it will exclude that
    condition and then it is just going to grab the first row from the table
    and return that. prisma/prisma#5149
    * feat(commerce): move pppApplied into helper
    * feat(commerce): move hasPPP check to helper
    * feat(commerce): return result instead of throwing
    * feat(commerce): move more PPP logic into helper
    * feat(commerce): consolidate PPP logic
    * cleanup(commerce): remove user purchases lookup
    * feat(commerce): combine PPP formatting scenarios
    * feat(commerce): move bulk coupon logic into helper
    * feat(commerce): try renaming incoming coupon
    The incoming `merchantCouponId` is a reference to a candidate coupon,
    e.g. maybe a site-wide coupon. We may override this with a better
    coupon, so we should treat it is as a candidate while we figure out what
    coupon will actually be applied.
    * feat(commerce): make coupon comparison more explicit
    * feat(commerce): find available coupons in helper
    * feat(commerce): lookup bulk coupon in helper
    * cleanup(commerce): remove unused PPP percent const
    * feat(commerce): apply bulk, combine conditions
    * feat(commerce): tighten fixed discount logic
    * feat(commerce): pull upgradeDetails logic up
    * feat(commerce): collapse bulk logic into rest
    * feat(commerce): collapse available PPP logic
    * feat(commerce): collapse extraneous 'special' logic
    The block toward the top of the price formatting that was catching a
    first pass of 'special' coupon logic is now redundant with the following
    logic that applies any coupons, including 'special', as needed.
    * feat(commerce): remove commented error-throwing
    * cleanup(commerce): clean up some comments
    * cleanup(commerce): more coupon comment cleanup
    * feat(commerce): reduce PPP details logic
    * feat(commerce): remove unused hasPurchaseWithPPP
    * feat(commerce): note future refactoring
    * feat(commerce): get bulk coupon, instead of details
    * feat(commerce): no need to return pppDetails
    The `pppDetails` served their purpose for the refactoring, but all that
    info is self-contained to the helper module and doesn't need to be
    passed to the price formatter anymore.
    * feat(commerce): remove coupon `code` logic branch
    The `code` to `Coupon.id` look up and translation happens in the
    `propsForCommerce` endpoint which then only passes along a coupon ID
    after that point. There is no consumer or app of the
    `formatPricesForProduct` code path that passes in a `code`.
    * feat(commerce): remove last redundant price block
    * fix(commerce): don't lookup coupon w/ undefined
    This is sorta weird Prisma behavior, if the main attribute of the where
    clause is `undefined` in a `findFirst` query, it will exclude that
    condition and then it is just going to grab the first row from the table
    and return that. prisma/prisma#5149
    * fix(commerce): ensure PPP does not leak through
    In the scenario where you had 'checked' the PPP discount box and then
    changed to Team/Bulk purchase (via toggle), the applied PPP merchant
    coupon would be carried over and applied as a fall-through value.
    This updates the logic to ensure PPP only gets assigned via the PPP code
    path and that special coupons are the ones that fall through.
    * fix(commerce): remove unused var, function call
    * feat(commerce): handle PPP + Bundle Upgrade
    This deals with linear issue TT-331

    It's a nightmare.
    You need to add a huge red banner with an explanation to the documentation!
    I can only imagine how many bugs appear because of this "smart" design

    Hi everyone, I don't think that Prisma team (I don't work at Prisma anymore) monitors closed issues (someone from the team, please correct me if I am wrong?).

    I do see that this issue is getting a lot of attention. To make progress, please create a new ticket, link it back to this one and show that a lot of community members want this changed 🙏🏼

    Thank you.

    No. Someone made this design decision a long time ago, we can only change it with a breaking change, and we are not eager to do that right now. We are aware it is a problem and are taking comments like these into account. But I am also aware that engaging here will not really make any of you happy, as you want to get the behavior reverted or a big red banner at the top of our docs - both which will not happen.

    There also a lot of other issues about this, for example #11021 or #20169. You can leave you upvotes on these.

    Appreciate the response. I figured that was the case and while not the ideal people in this thread want, I understand, because yeah.... this would be a massive breaking change. Appreciate ya chiming in here so that way in the future people at least know the current state on this from Prisma's perspective!

    I still believe this is such a dangerous design: once again just imagine writing auth function that looks up user by email and hash and by accident it comes up empty. The first user is usually the admin…

    It would be worth it to change it with breaking change and add configuration option to opt in to the old behavior.

    It’s just dangerous and not only bad design. For now the big red alert anywhere near filters documentation should be posted.

    Respectfully this is a ticking time bomb. It's a nasty post-mortem waiting to go viral on HN. We've been using Prisma for over a year at this point and have read a good chunk of this documentation. Somehow we're only learning about this behavior now. I'm guessing the same is true for a lot of other users.