search

gquintard / vmod-reqwest

BSD 3-Clause "New" or "Revised" License
9 stars 7 forks source link

Acces cookie data fetched from request #5

Closed Aadarsh-Verma closed 1 year ago

Aadarsh-Verma commented 1 year ago

Cannot find a way to access the cookie data fetched from the request made , If vmod_reqwest does not support it please let me know how can I accomplish the same using any other vmod I am using the below syntax for making the API call :

new client = reqwest.client();

        client.init("get_visitor_id", "URL");
        client.send("get_visitor_id");
gquintard commented 1 year ago

Hi,

there's the .get_header() method you can use: https://github.com/gquintard/vmod_reqwest/blob/main/vmod.vcc#L78-L80

You'd use it like this:

        client.init("get_visitor_id", "http://api.example.com");
        client.send("get_visitor_id");
        set req.http.my-cookie = client.header("get_visitor_id", "set-cookie");

please let me know if that solves your need

Aadarsh-Verma commented 1 year ago

But I have multiple Set-Cookie key value pairs and it picks only the first one and ignores the rest Is there a way all the values can be extracted and then set to the cached response

gquintard commented 1 year ago

Ah, you are correct! I will add a way to collect all the headers

On Wed, Feb 22, 2023, 00:02 Aadarsh Verma @.***> wrote:

But I have multiple Set-Cookie key value pairs and it picks only the first one and ignores the rest Is there a way all the values can be extracted and then set to the cached response

— Reply to this email directly, view it on GitHub https://github.com/gquintard/vmod_reqwest/issues/5#issuecomment-1439580822, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA42AKNNFNCG6RDP3GW2BGDWYXB2LANCNFSM6AAAAAAVDHPU74 . You are receiving this because you commented.Message ID: @.***>

Aadarsh-Verma commented 1 year ago

Thanks a Lot !!! Can u give an estimate by when you can add that function so that I can look for another solution or wait as per my timeline.

gquintard commented 1 year ago

most probably this weekend, it's just a matter of:

not a huge time investment, I just need to find a slot. Alternatively, if somebody on your side wants to get their feet wet with rust, I'll happy to mentor and/or review a PR ;-)

gquintard commented 1 year ago

@Aadarsh-Verma, please have a look at the latest main commit (https://github.com/gquintard/vmod_reqwest/commit/6d7d53a6e7c00f1920b1b96261743b814bce5743), you can now specify sep to tell vmod_reqwest to collect the headers in a single string. I assume in your case that should be something like

        client.init("get_visitor_id", "http://api.example.com");
        client.send("get_visitor_id");
        set req.http.my-cookie = client.header("get_visitor_id", "set-cookie", sep = "; ");

please let me know if that covers it, and of course reopen this issue if it doesn't

Aadarsh-Verma commented 1 year ago

Using This Dockerfile ` FROM rust:1.61-buster

WORKDIR /vmod_reqwest ARG VMOD_REQWEST_VERSION=0.0.6

ARG RELEASE_URL=https://github.com/gquintard/vmod_reqwest/archive/refs/tags/v${VMOD_REQWEST_VERSION}.tar.gz

ARG RELEASE_URL=https://github.com/gquintard/vmod_reqwest.git

RUN curl -s https://packagecloud.io/install/repositories/varnishcache/varnish72/script.deb.sh | bash && apt-get update && apt-get install -y varnish-dev clang libssl-dev git

RUN git clone https://github.com/gquintard/vmod_reqwest.git && \ cd vmod_reqwest && \ cargo build --release

FROM varnish:7.2 COPY --from=0 /vmod_reqwest/target/release/libvmod_reqwest.so /usr/lib/varnish/vmods/

USER root

RUN apt-get update && apt-get install -y net-tools curl`

Getting the error below :

10 112.7 --> /usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/varnish-0.0.14/src/vcl/backend.rs:73:5

10 112.7 |

10 112.7 73 | use std::ffi::c_char;

10 112.7 | ^^^^^^^^^^^^^^^^ no c_char in ffi

10 112.7

10 112.7 error[E0432]: unresolved imports std::ffi::c_char, std::ffi::c_uint

10 112.7 --> /usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/varnish-0.0.14/src/vcl/ctx.rs:3:16

10 112.7 |

10 112.7 3 | use std::ffi::{c_char, c_uint, c_void};

10 112.7 | ^^^^^^ ^^^^^^ no c_uint in ffi

10 112.7 | |

10 112.7 | no c_char in ffi

10 112.7

10 112.7 error[E0432]: unresolved imports std::ffi::c_char, std::ffi::c_uint

10 112.7 --> /usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/varnish-0.0.14/src/vcl/http.rs:15:16

10 112.7 |

10 112.7 15 | use std::ffi::{c_char, c_uint};

10 112.7 | ^^^^^^ ^^^^^^ no c_uint in ffi

10 112.7 | |

10 112.7 | no c_char in ffi

10 112.7

10 112.7 error[E0432]: unresolved imports std::ffi::c_char, std::ffi::c_int

10 112.7 --> /usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/varnish-0.0.14/src/vcl/processor.rs:8:16

10 112.7 |

10 112.7 8 | use std::ffi::{c_char, c_int, c_void};

10 112.7 | ^^^^^^ ^^^^^ no c_int in ffi

10 112.7 | |

10 112.7 | no c_char in ffi

10 112.7

10 112.7 error[E0432]: unresolved import std::ffi::c_char

10 112.7 --> /usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/varnish-0.0.14/src/vcl/ws.rs:13:16

10 112.7 |

10 112.7 13 | use std::ffi::{c_char, c_void};

10 112.7 | ^^^^^^ no c_char in ffi

10 112.7

10 112.9 For more information about this error, try rustc --explain E0432.

10 112.9 error: could not compile varnish due to 5 previous errors

10 112.9 warning: build failed, waiting for other jobs to finish...

Attatched the whole build result over here :

https://paste.sh/ZkTLMY99#uoiLk9mGcO7anYlVtlonunut

Let me know if I am doing something wrong

gquintard commented 1 year ago

thanks, that's interesting, I can reproduce with your Dockerfile, but not on my machine. It's possible a cargo version issue or something

gquintard commented 1 year ago

confirmed, if you change your first line to FROM rust:1.67-buster you should be fine.

I'll keep this issue open until you can confirm

Aadarsh-Verma commented 1 year ago

Thanks a lot !!! This is working fine I am not facing the compilation errors on updating the rust version However there is a small change
` FROM rust:1.67-buster

WORKDIR /vmod_reqwest

ARG VMOD_REQWEST_VERSION=0.0.6

ARG RELEASE_URL=https://github.com/gquintard/vmod_reqwest/archive/refs/tags/v${VMOD_REQWEST_VERSION}.tar.gz

ARG RELEASE_URL=https://github.com/gquintard/vmod_reqwest.git

RUN curl -s https://packagecloud.io/install/repositories/varnishcache/varnish72/script.deb.sh | bash && apt-get update && apt-get install -y varnish-dev clang libssl-dev git

RUN git clone https://github.com/gquintard/vmod_reqwest.git && \ cd vmod_reqwest && \ cargo build --release

FROM varnish:7.2

COPY --from=0 /vmod_reqwest/target/release/libvmod_reqwest.so /usr/lib/varnish/vmods/

COPY --from=0 /vmod_reqwest/vmod_reqwest/target/release/libvmod_reqwest.so /usr/lib/varnish/vmods/

USER root RUN apt-get update && apt-get install -y net-tools curl`

the libvmod_reqwest.so file is in this directory /vmod_reqwest/vmod_reqwest/target/release/libvmod_reqwest.so instead of the directory mentioned in the official Dockerfile

Aadarsh-Verma commented 1 year ago

Also needed your help with something related to varnish I am trying to make varnish ignore headers and cookies from including in the hash_data without unsetting them but I cannot find any solution for the same , Can u suggest a documentation that I can go through that would help me Also a platform where I can discuss the issues that I am facing generally on varnish

gquintard commented 1 year ago

ah, thanks, changing WORKDIR /vmod_reqwest to WORKDIR / should fix the issue, but I'll check and correct, thank you for reporting it.

Does the new function work as expected?

Regarding hashing, that should be fairly easy as adding anything to the hashing key is done explicitly via hash_data (https://varnish-cache.org/docs/trunk/users-guide/vcl-hashing.html), so you don't need to unset the header, just to not call hash_data on it. If I'm not making any sense, I'd be happy to discuss that over on the discord channel (https://varnish-cache.org/support/). I'm in the PST timezone, so our schedule may not match, but there are plenty of good folks ready to help

Aadarsh-Verma commented 1 year ago

Does the new function work as expected? yes it's working as expected.