From 7aa7aa7ebaf536a7222428e351a887214d3d63de Mon Sep 17 00:00:00 2001 From: CahidArda Date: Mon, 21 Oct 2024 08:49:51 +0300 Subject: [PATCH 1/7] fix: add tests for ratelimit with protection --- src/deny-list/ip-deny-list.test.ts | 34 +++++++++++++++++++++++++++++- src/deny-list/ip-deny-list.ts | 1 + src/hash.test.ts | 24 +++++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 src/hash.test.ts diff --git a/src/deny-list/ip-deny-list.test.ts b/src/deny-list/ip-deny-list.test.ts index 314610c..3d32198 100644 --- a/src/deny-list/ip-deny-list.test.ts +++ b/src/deny-list/ip-deny-list.test.ts @@ -3,6 +3,7 @@ import { beforeEach, describe, expect, test } from "bun:test"; import { checkDenyList } from "./deny-list"; import { disableIpDenyList, updateIpDenyList } from "./ip-deny-list"; import { DenyListExtension, IpDenyListKey, IpDenyListStatusKey } from "../types"; +import { Ratelimit } from ".."; describe("should update ip deny list status", async () => { const redis = Redis.fromEnv(); @@ -177,4 +178,35 @@ describe("should only allow threshold values from 1 to 8", async () => { expect(error.name).toEqual("ThresholdError") } }) -}) \ No newline at end of file + + test.only("should be able to use ratelimit with deny list", async () => { + + /** + * When enableProtection is set to true, there was an error in the + * @upstash/ratelimit 2.0.3 release. Because the @upstash/redis + * uses auto-pipelining by default, the client would send the + * EVALSHA and the protection EVAL at the same time. + * + * When the db loses its scripts after a SCRIPT FLUSH or when + * ratelimit is used in a new DB, the EVALSHA will fail. But + * because since the EVAL and EVALSHA are pipelined, they will + * both fail and EVAL will get the EVALSHA's error. + */ + const redis = Redis.fromEnv({ enableAutoPipelining: true }); + + // flush the db to make sure + await redis.scriptFlush() + // sleep for two secs + await new Promise(r => setTimeout(r, 2000)); + + const ratelimit = new Ratelimit({ + limiter: Ratelimit.fixedWindow(2, "1s"), + redis, + enableProtection: true + }) + + const { success, remaining } = await ratelimit.limit("ip-deny-list") + expect(success).toBeTrue() + expect(remaining).toBe(1) + }) +}) diff --git a/src/deny-list/ip-deny-list.ts b/src/deny-list/ip-deny-list.ts index 34efebf..8a15f51 100644 --- a/src/deny-list/ip-deny-list.ts +++ b/src/deny-list/ip-deny-list.ts @@ -83,6 +83,7 @@ export const updateIpDenyList = async ( // delete the old ip deny list and create new one transaction.del(ipDenyList) + // @ts-expect-error ...allIps type works but gets type error transaction.sadd(ipDenyList, ...allIps) // make all deny list and ip deny list disjoint by removing duplicate diff --git a/src/hash.test.ts b/src/hash.test.ts new file mode 100644 index 0000000..c6ff537 --- /dev/null +++ b/src/hash.test.ts @@ -0,0 +1,24 @@ +import { Redis } from "@upstash/redis"; +import { describe, test } from "bun:test"; +import { safeEval } from "./hash"; +import { SCRIPTS } from "./lua-scripts/hash"; + +const redis = Redis.fromEnv(); + +describe("should set hash correctly", () => { + test("should set hash in new db correctly", async () => { + await redis.scriptFlush() + + // sleep for two secs + await new Promise(r => setTimeout(r, 2000)); + + await safeEval( + { + redis + }, + SCRIPTS.singleRegion.fixedWindow.limit, + ["id"], + [10, 1] + ) + }) +}) \ No newline at end of file From 658e3d2bef8baff8da2f6195dc754523b524187a Mon Sep 17 00:00:00 2001 From: CahidArda Date: Mon, 21 Oct 2024 08:53:38 +0300 Subject: [PATCH 2/7] fix: bump redis --- bun.lockb | Bin 117821 -> 118145 bytes examples/cloudflare-workers/package.json | 4 ++-- examples/nextjs/app/api/route.ts | 1 - examples/nextjs/package.json | 6 +++--- package.json | 2 +- src/types.ts | 24 ++++++++--------------- 6 files changed, 14 insertions(+), 23 deletions(-) diff --git a/bun.lockb b/bun.lockb index 54f18a44e2ef1e521aef4b057134e5a8f8e3b2de..665316b9619caf20cf53886744ce67bd21c18552 100755 GIT binary patch delta 3982 zcmc(ik9Sk$8OQHiOlYdD3`;}7hC;g_Rsx0E6k38(iMoSf&O-o&PGn2LKs;wcM1htc zngP=a+?lI5q8pkGoe}Mp4jBRF7N$ZN%w_xxuyd?)oAt1r(<#oJJ>PF{nceD}1lKVdSSLUDcgSt}FeTtw{>F@fiKqiW+@l zX`!u2hR+dOsH@p-+XA`t?N@r|rMK`(ujyV*yXQ*pY3a@E?^)iL-YxpHibh-MeVT^5 zSafx}+xB(oO_iQaUw@<9wnchg=@sazs-e&<4bl%ayKO}a`cAt_SEJlEuk`9<;2{0h zv2NRAW^J&(zQS!wO7A}tzFOutTSoRx|6b``nUs2=^U*~?+h{!RV!_oCLlUhLS4b?B zxJF`uUL3UFB^|9@dh+8dLR>GfNQ_7uk++dq()9>Tgv^okt6)0FZqoB8OqA>)y^q1f$X?R78YWKWN&gy{ z1ZiE1yoF4XC2cSPGDViP!-U8*=~xF7CNrdSJxm*!C0!d}B4m!N-w4w|c9WjRVWMOY z>3sqwM)s1v2uz&Jlm73*BuMKfU$S>FNEL3Wd#r(vRG59!?s6C-;`-!_;ynJ4|*VG^WON8Umv$&&BG1jrOw_6$sj zOp}f%Oqk4&P6MWm%#yBWVIpLXtbY!sgX|_fJ7A(@59xg#CPwy>z8}EE$vo-b36mhL zG2|^|k}P=vCP1dhvKL`OWSVsR5GG7!Nav4W+Q=;F+65CKb7cKXFdbw!>3JC@O7@W6 zI82P}C4E1JiIaKK{}Y%5Y2A&yg-ntqufPPz6j}BvOo&XAj-SGW$qeay4W^CElCA_y zgv^okufueZ-K1v^OqA>)y>Gz8$X?R-CQO{nlm1Sa1Zn*l@)j~lmi!zhK&HsDBut1* zla4N!Fqt8pzkq2Yv!rV;OoYsl^>4v+klmzbA54_&A-%tZiIKgeZ$C_&%#;2UOoFr? zK;A+o$&!OG0Ww9F9fApwY0~j4m@t_koo~amky+C9YnTX`BkO+y(?NEVo-|C9>><5} zVPa%2>H95AoXnH{cVH5v^%mj>3e=4C&0kw2@iTbqpp# z=E(Zr!E}(_q~|zHlBo&Q&tbxK6Gw$CLdU7KfLDR%=05QAHVIJ zqvmgT_>+$}|9M&Vv&7@Gg5SFEx5UiG)x%0(FAaH9T_;xZfAIXZd5L57DW_-2Epn&r z`p(n0T=S*UIQ=7*I?mYjyUq+K)#%Ti!C9WOcKwTirSbZmXHn{r(raf2oHarJjHTE) zyT0(;fKsi#`y9^l-%M?`26{{6^=z z3zhcmrKO##{<*kGA9As_sPblcOgYZFjSnWP>Bi_fRWl4bA{=FW1)4TU@&}DYb*i&n z?;EaFO2k&ESlS@djWLJI-c`Fv^1%|TOn0s1frP5ZJ?nZOmFmVed$5CUtGRg0eynmu7+oer3$6=yK zW&5nj)|ZEiuAMt^?QT;}Fb9s1{u7d8Qn-lxw2{*0NP?Tyn(lhZC!;9GB-6=Wof}JB z12vQ#(x#Z>>LlXfWXG>f#RXBUmxxuVl;)8fMVuf%ZM3xMrkiTIG2&)OUMaEBbcGF) z$nj~D;b>B0j7qnvHtuk%%A%kYmP>m;b%orjNLjF<)w|X z8M_*q&8UE@jM5l zxkcjhc~^2g=gSiDm10HlGG3HER{Rx-SkZnz&aA=zMcN{1SPZNjuGc^ICG$M2G8S-x z^q1nRTP(@_((wIZ#jry1#|c-ZMPmO2a06TrlfzBncZtK~`X{lcSvqo@SZ3^)p`3U& z7cQ(?ifMJvAj@CvQ#A2ZaUs^K;nCU11z#AQ$IseWt6Ukg-AookY7qT%o0Z`L@hDzKkz)m zvs7l&cd4%LEL9Eazv{UMrQSF5zDbvx_b|s9_XlM_-^{;!_o4j{P>JtQjte2)Mr0B()Fk1YQ{X{Qb>(7PV7{bhU=iR8b=PRVq<(r6?ZkK)n&`S0Zumw Ap8x;= delta 3849 zcmc(i|96vD9mk(rXxem;wNTP^TSEpceCY^uZIiNfp%xeBj)ueG5MYyd@JQD3CH$~M zhI~m`9Rb`HzC756ISnYIlPPhmnmU!R%^AjU8B-Ve8bFUd>JL$Vu@24Rr%N2hI1SK{i<4cV@K!0Y{R+E-OIA4 zw+{PH<^Fj5tkTfD`<6{EyVQJQQ=4_>1Q|OuZF|KEO{K*hyJ;Shpp!&YZqUx*DUl}YoFA#hh${c_s#NJ12XcXQ*_75csguXVGG{=4E2Wwc0YBkOm_ za&N7wZQ#tRwbq_HiF<93PVGTtE(^LCnk zml=1P{&|UGrM^cZu0No(ft#!5{ofa%Z`t^S()9esWkPopjFSb@^9)Ra93p+2VEV`+ z>HjiJk{l)jn_>FN5*geAlOio!k%!1MY1;-9CNrcx1`{E(q@xEWO6Ew{c94iyGl+2N?=V4-Go^-ze6DJF#=S7$VIYjyrFnwf^^zVU5lEY+RFHApKB7^&2Qlw=+ z@(`IOZ8}Vt%#ij2FcC6KI=%)IC3B>!4<<(DNw)zLCkv$KAWVWBB7HBx^pQo<|8Dbn&yU`={O7%C3B?fTQD&)PrAPi6DJF# z=Lk%K93p+M!1R$t(ti{tNe+{Newco;Lw7RUGEcgX!^Ft~=}EyP$RX1AeV9J7NcvxgNs_~4-~>!RSt5gPz@$ja0P+x- zCT%}}36mMp{zI4unI#=*m?)VeU4t+&GEcgH1QRC}vzkrF7 zInwn3OpMHv?i@^H23beO5}oOLhA3OQT9N^?$Hbl+wz}qe_kXvCEj1yj-V`zcQ*cTVHVnrNEUs{p9G< z9Nk(#sU)TKg;BGb^fy^b73%bctD{QIdiPb#3SF(!KNwwV)$6aJWV=?UZ@bo^f9m>@ zf$HndI=%Y(*n!9X{)P4`XN!DCX@Bt;zim86_QPH_)NULkRs>8d54ZC-c3>1xDnkdEdu9ZsvuXtW+} zx+y1$kmDWO4Aa%hC@MMjZ>B^naFXXl3<}&6r3D#wYf;7 zx#@a%Ue@GL%QnZn@NO9`HXSBl&OOqflLU)uGTptBFO?i0_MqwHM6ESTj)sWTAsq)r z+yff~C+Bi)9!`^JtrGF=oYEhX97V50?9_eIeUdvRq9*^eDbnYg?qSnS71t^GeG=PE z=fq8<)j~+CYTwxWpwHRje7bw{>J8xsjDLDnwNcrorj>nB8ME5d6w5lfvPv5awy7#r zj#KOY!PWEB&9ZWwh_@K4+tpMnHU*ntGhS?0_gH%+#iNNCC)!orczn~K^fu``jgMtQ zRfqZcdpnIGuY3qzg)(i>{fO!-b%p(HZ`M%_M7)K@INnzXZ6c5W7ugYr@XMMuY zFm80JiT7jUu*?ih5eq3g}mU^&&ON+3aqTw?lqgS>?CL zyDR4Yt?GsJo-$n7;-0H%A>(GZsyD`Usk_x0qhX!o9mso)C%aU=I%L#HS!IkpU8?ck zg4{;_E8YHq7rrq6jis~OeqXKryxholDW`hMSS+^~HLYIt7>CxW#&XU!E_A5|V}G~m zQX?NddW!Mb8u{>%y9{n!quSK})c>$n>gLRm^^V-We&o95k)_`H%n7$vd>7s)Y3J7) mm)EIJ4?5PXcT`nnWW*jlJNT!F>RfKzaYjuT+(key: string, ...members: TData[]) => Promise; + sadd: RedisCore["sadd"] - hset: (key: string, obj: { [key: string]: TValue }) => Promise; + hset: RedisCore["hset"] - eval: ( - ...args: [script: string, keys: string[], args: TArgs] - ) => Promise; + eval: RedisCore["eval"] - evalsha: ( - ...args: [sha1: string, keys: string[], args: TArgs] - ) => Promise; + evalsha: RedisCore["evalsha"] - scriptLoad: ( - ...args: [script: string] - ) => Promise; + scriptLoad: RedisCore["scriptLoad"] - smismember: ( - key: string, members: string[] - ) => Promise; + smismember: RedisCore["smismember"] - multi: () => Pipeline + multi: RedisCore["multi"] } From 2973d39bb870649de56e4d6d2575df6bbd63c2e1 Mon Sep 17 00:00:00 2001 From: CahidArda Date: Mon, 21 Oct 2024 08:54:10 +0300 Subject: [PATCH 3/7] ci: run local nextjs/cf without waiting for tests --- .github/workflows/tests.yaml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index af15231..cafb5f8 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -39,8 +39,6 @@ jobs: run: bun run test cloudflare-workers-local: - needs: - - test runs-on: ubuntu-latest steps: - name: Setup repo @@ -127,8 +125,6 @@ jobs: DEPLOYMENT_URL: https://upstash-ratelimit.upsdev.workers.dev nextjs-local: - needs: - - test runs-on: ubuntu-latest steps: - name: Setup repo From 92a2268fc00fbdd1514f99a1985dcf07a9875146 Mon Sep 17 00:00:00 2001 From: CahidArda Date: Mon, 21 Oct 2024 09:10:00 +0300 Subject: [PATCH 4/7] ci: add install ci version for nextjs-deployed --- .github/workflows/tests.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index cafb5f8..58e4734 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -191,8 +191,8 @@ jobs: VERCEL_PROJECT_ID: "prj_NSOSq2ZawugKhtZGb3ViHX4b56hx" working-directory: examples/nextjs - - name: Install dependencies - run: bun install + - name: Install @upstash/ratelimit canary version + run: npm install @upstash/ratelimit@${{needs.release.outputs.version}} working-directory: examples/nextjs - name: Build Project From 2cd2f3cb8c572d25e2b3c74aeeab6f4299af09c7 Mon Sep 17 00:00:00 2001 From: CahidArda Date: Mon, 21 Oct 2024 11:26:23 +0300 Subject: [PATCH 5/7] fix: rm expect error --- src/deny-list/ip-deny-list.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/deny-list/ip-deny-list.ts b/src/deny-list/ip-deny-list.ts index 8a15f51..dd03f3a 100644 --- a/src/deny-list/ip-deny-list.ts +++ b/src/deny-list/ip-deny-list.ts @@ -83,8 +83,8 @@ export const updateIpDenyList = async ( // delete the old ip deny list and create new one transaction.del(ipDenyList) - // @ts-expect-error ...allIps type works but gets type error - transaction.sadd(ipDenyList, ...allIps) + + transaction.sadd(ipDenyList, allIps.at(0), ...allIps.slice(1)) // make all deny list and ip deny list disjoint by removing duplicate // ones from ip deny list From 8b0dcd88b467af223236d9c3300c7655ccef59d8 Mon Sep 17 00:00:00 2001 From: CahidArda Date: Mon, 21 Oct 2024 16:47:22 +0300 Subject: [PATCH 6/7] fix: tests --- src/deny-list/ip-deny-list.test.ts | 62 +++++++++++++++--------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/src/deny-list/ip-deny-list.test.ts b/src/deny-list/ip-deny-list.test.ts index 3d32198..3314471 100644 --- a/src/deny-list/ip-deny-list.test.ts +++ b/src/deny-list/ip-deny-list.test.ts @@ -140,6 +140,37 @@ describe("should update ip deny list status", async () => { expect(newStatus).toBe("valid") expect(newStatusTTL).toBeGreaterThan(1000) }) + + test("should be able to use ratelimit with deny list", async () => { + + /** + * When enableProtection is set to true, there was an error in the + * @upstash/ratelimit 2.0.3 release. Because the @upstash/redis + * uses auto-pipelining by default, the client would send the + * EVALSHA and the protection EVAL at the same time. + * + * When the db loses its scripts after a SCRIPT FLUSH or when + * ratelimit is used in a new DB, the EVALSHA will fail. But + * because since the EVAL and EVALSHA are pipelined, they will + * both fail and EVAL will get the EVALSHA's error. + */ + const redis = Redis.fromEnv({ enableAutoPipelining: true }); + + // flush the db to make sure + await redis.scriptFlush() + // sleep for two secs + await new Promise(r => setTimeout(r, 2000)); + + const ratelimit = new Ratelimit({ + limiter: Ratelimit.fixedWindow(2, "1s"), + redis, + enableProtection: true + }) + + const { success, remaining } = await ratelimit.limit("ip-deny-list") + expect(success).toBeTrue() + expect(remaining).toBe(1) + }) }) describe("should only allow threshold values from 1 to 8", async () => { @@ -178,35 +209,4 @@ describe("should only allow threshold values from 1 to 8", async () => { expect(error.name).toEqual("ThresholdError") } }) - - test.only("should be able to use ratelimit with deny list", async () => { - - /** - * When enableProtection is set to true, there was an error in the - * @upstash/ratelimit 2.0.3 release. Because the @upstash/redis - * uses auto-pipelining by default, the client would send the - * EVALSHA and the protection EVAL at the same time. - * - * When the db loses its scripts after a SCRIPT FLUSH or when - * ratelimit is used in a new DB, the EVALSHA will fail. But - * because since the EVAL and EVALSHA are pipelined, they will - * both fail and EVAL will get the EVALSHA's error. - */ - const redis = Redis.fromEnv({ enableAutoPipelining: true }); - - // flush the db to make sure - await redis.scriptFlush() - // sleep for two secs - await new Promise(r => setTimeout(r, 2000)); - - const ratelimit = new Ratelimit({ - limiter: Ratelimit.fixedWindow(2, "1s"), - redis, - enableProtection: true - }) - - const { success, remaining } = await ratelimit.limit("ip-deny-list") - expect(success).toBeTrue() - expect(remaining).toBe(1) - }) }) From 0dfb86bf661cfec6539865f981e871d7ada88963 Mon Sep 17 00:00:00 2001 From: CahidArda Date: Mon, 21 Oct 2024 17:08:15 +0300 Subject: [PATCH 7/7] fix: test expected evalsha count --- src/cache.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cache.test.ts b/src/cache.test.ts index e4564ac..93ffb1c 100644 --- a/src/cache.test.ts +++ b/src/cache.test.ts @@ -5,6 +5,7 @@ import { Ratelimit } from "./index"; test("ephemeral cache", async () => { const maxTokens = 10; const redis = Redis.fromEnv(); + await redis.scriptFlush() const metrics: Record = {}; @@ -37,7 +38,7 @@ test("ephemeral cache", async () => { } expect(passes).toBeLessThanOrEqual(10); - expect(metrics.evalsha).toBe(11); + expect(metrics.evalsha).toBe(12); expect(reasons).toContain("cacheBlock") await new Promise((r) => setTimeout(r, 5000));