Add strict mode to validate protocol strings

[v0y4g3r]

For performance reasons, we expect requests from the remote write protocol to be valid protobuf bytes. We should provide a strict mode to perform UTF-8 validation.

Originally posted by @evenyag in #3425 (comment)

[evenyag]

We could also use the bstr crate if possible...

[carmooo]

Hi, would be first time contributor here 👋
Is it ok if I give it a go?
Any more info you might have that would be useful?
Thanks!

[tisonkun]

@carmooo

You may search "// safety: we expect all labels are UTF-8 encoded strings." in the code and this issue is about add a strict mode to check the value of PromLabel are valid UTF-8 strings.

It would require:

1. Where to add the strict mode config/param? @v0y4g3r @evenyag @waynexia do you have some suggestions here?
2. How to efficiently validate payload. @evenyag suggests you take a look at the bstr crate also.

What's your plan to handle this issue? I expect to listen to your plan; bare assignment and waiting until stale is not a good practice.

[carmooo]

Thanks for the info @tisonkun.
My plan is to check if the bstr crate is a valid option for this validation, if so I'll use it for validation. I also need to figure out where to add the strict mode configuration so as to make the validation conditional.
Does that answer your question?
Or would you like any more info?
Thanks!

[evenyag]

We could add a strict_mode option in the HttpOptions and pass it to the prom server.

greptimedb/src/servers/src/http.rs

Lines 115 to 127 in 74862f8

	#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
	#[serde(default)]
	pub struct HttpOptions {
	pub addr: String,
	#[serde(with = "humantime_serde")]
	pub timeout: Duration,
	#[serde(skip)]
	pub disable_dashboard: bool,
	pub body_limit: ReadableSize,
	}

Using bstr might be more complicated than I expected as both rust String and arrow StringArray use utf-8 encoding.

[tisonkun]

@carmooo though you almost repeat my comment ... it's the way to go! Lol.

Do not hesitate to ask here if you stall by any uncertaincy. And you can send a draft PR when a rough impl is ready so that we can involve early.

[evenyag]

My plan is to check if the bstr crate is a valid option for this validation, if so I'll use it for validation.

bstr treats byte strings as conventionally UTF-8 but doesn't speed up utf-8 validation. So we still need to convert it into utf-8 string unless we use bstr as the default string implementation.

@carmooo I'd suggest adding an option to enable the validation as it is much easier to implement. You could pass the strict mode flag to the write_fast() method.

greptimedb/src/servers/src/query_handler.rs

Lines 100 to 106 in 74862f8

	/// Handling prometheus remote write requests
	async fn write_fast(
	&self,
	request: RowInsertRequests,
	ctx: QueryContextRef,
	with_metric_engine: bool,
	) -> Result<()>;

[carmooo]

@carmooo though you almost repeat my comment ... it's the way to go! Lol.

Yes, I did indeed @tisonkun 😅
I'll try to get a draft MR ready asap and will ask questions about any blockers.
@evenyag I'll follow your suggestions

[carmooo]

Sorry for backing out but cannot take care of this right now.

[tisonkun]

Never mind. Thanks for your time and sharing :D

[irenjj]

can i take it？

[tisonkun]

@irenjj Of course you can take it. But the same as above, without certain progress, bare assignment doesn't help.

You can ask any questions or submit a patch without an assignment.

[tisonkun]

Assigned. But I'd still clarify that "Can I take it" doesn't help a lot. Everyone is welcome to take over an issue here so it's not even a question. You can ping me once a (draft) PR is available or any certain questions.

[evenyag]

Closed by #3638