Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

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

RFC: an object-oriented vmod_blob? #4255

Open
nigoroll opened this issue Jan 16, 2025 · 10 comments
Open

RFC: an object-oriented vmod_blob? #4255

nigoroll opened this issue Jan 16, 2025 · 10 comments

Comments

@nigoroll
Copy link
Member

Asking for feedback: Should we have a different interface to the encode/decode functions provided by vmod_blob? Depending on feedback, I would turn this into a VIP or not.

Quick idea draft:

$Module codec

# decoded form is a blob
$Object blob(ENUM {IDENTITY, BASE64, ...} encoding, ENUM {LOWER, UPPER, DEFAULT} case="DEFAULT"}
$Method STRING encode(BLOB)
$Method BLOB decode(STRANDS)

# decoded form is a string
$Object string(ENUM {IDENTITY, BASE64, ...} encoding, ENUM {LOWER, UPPER, DEFAULT} case="DEFAULT"}
$Method STRING encode(STRING)
$Method STRING decode(STRANDS)
sub vcl_init {
	new myurl = codec.string(URL, LOWER);
}

sub vcl_recv {
	set req.url = "/?url=" + myurl.encode(req.url);
}
``
@dridi
Copy link
Member

dridi commented Jan 17, 2025

To the question of object-oriented blobs I would wholeheartedly approve, but not so much in vmod_blob.

At some point I was working with @walid-git on enriching the type system to make the built-in types more powerful and in some cases make vmods obsolete (or reduce their scope).

The plan was roughly:

  • ✔ Document types in a new vcl-types(3) manual page
  • ✔ Generate the C code backing types and their methods/attributes from docs
  • Restructure methods to take arguments (same JSON as VMODs essentially)
  • Add new methods to types
  • Consider new types for existing symbols

With your example, we could imagine something like this:

  • req.url has a new URL type
  • URL type has $Method STRING encode(ENUM {lower, upper, default} case="default")

And you could do:

set req.url = "/?url=" + req.url.encode(lower);

The items with check marks in the plan were partially implemented but I don't remember where we stopped or what was left to do to drive the current type system from documentation like we do for variables.

My 2 cents.

@nigoroll
Copy link
Member Author

@dridi thank you for this interesting perspective! Too bad that your initiative did not get further yet.

I have two questions on your comment:

  • Would it really make sense to tie the proposed .encode() method to a new url type? Would this not be one for STRING/STRANDS and BLOB?
  • What are your thoughts on how VMODs could extend built-in types?

@dridi
Copy link
Member

dridi commented Jan 17, 2025

@dridi thank you for this interesting perspective! Too bad that your initiative did not get further yet.

I'm sure we'll find time to revisit this with @walid-git. He wrote all the code so far so he probably remembers better than me where it's currently at.

Would it really make sense to tie the proposed .encode() method to a new url type? Would this not be one for STRING/STRANDS and BLOB?

In this case I suggested a URL type because it has potential for dedicated methods and attributes:

  • URL.path
  • URL.query
  • URL.encode()
  • URL.tostring() /* no-op to change type */
  • ...

It can be simply backed by a const char * like the STRING type, and we could then have expressions like:

resp.http.location.tourl().path

What are your thoughts on how VMODs could extend built-in types?

I'm pretty sure I have already said publicly that I was in favor, where a function taking a type as its first argument could turn into a method. I haven't given much thought to designing that and at first glance it looks like a tough nut to crack open.

@nigoroll
Copy link
Member Author

@dridi thank you.

Regarding the URL type, I get your point regarding .path, .query etc (I want .param(name)), but still I think .encode() should be a method on string-ish. URL can also have it, but not only it. We should not limit .encode() to URL-encoding.

Regarding the first argument idea, yes, you did mention that at some point and I forgot.

@nigoroll
Copy link
Member Author

nigoroll commented Jan 17, 2025

Oh, @dridi, after my first impression of your idea being alternative to the suggestion herein, are they not actually complementary?

sub vcl_init {
	new percent = codec.string(URL, LOWER);
}

sub vcl_recv {
	set req.url = "/?url=" + req.url.percent.encode();
}

@dridi
Copy link
Member

dridi commented Jan 17, 2025

The problem is symbol collisions. If a percent attribute or method already existed, this would become ambiguous.

@dridi
Copy link
Member

dridi commented Jan 17, 2025

are they not actually complementary?

Yes they could be, but I would rather invest first in richer built-in types.

@nigoroll
Copy link
Member Author

If a percent attribute or method already existed

then name it differently?

@dridi
Copy link
Member

dridi commented Jan 20, 2025

When we add new symbols to VCL there's always the risk of breaking third-party VMODs. For example when the variables remote.* and local.* got introduced, hypothetical vmod_remote and vmod_local VMODs could no longer be imported.

I'm fine with that kind of breakage.

If we make our type system richer, chances are that we may more easily add attributes or methods to types than we add new global symbols. That would create very subtle VCL breakage caught somewhat late during VCL compilation (unlike a failed import statement) because a new symbol could magically replace another with a new release.

@nigoroll
Copy link
Member Author

@dridi at least we have import ... as ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants