Original fields are making search noise

[Ionys320]

📑 I have found these related issues/pull requests

Not found any related issue.

🛡️ Security Policy

• I agree to have read this project Security Policy

Description

When switching from a monitor type to another, the fields are not cleaned, meaning irrelevant fields may be used during the search process.

For example, in PR #5438, because every monitor as 1.1.1.1 as default DNS resolve server, searching for 1.1.1.1 will find all the monitors (or almost).

👟 Reproduction steps

1. Create an HTTP monitor with google.fr as the URL (no need to save) ;
2. Edit it to became a ping on cloudflare.com and save it ;
3. Search for google.fr in the searchbar of the monitors list.

👀 Expected behavior

No monitor should be displayed.

😓 Actual Behavior

The ping monitor is displayed, because the URL still contains google.fr.

🐻 Uptime-Kuma Version

2.0.0-beta.1 with changes of #5438 (note: this issue is only since this PR, because it extends the fields used for the search. That's why the last beta doesn't have it).

💻 Operating System and Arch

Windows 11 x64

🌐 Browser

Vivaldi 7.0.3495.27 (Stable channel) (64 bits)

🖥️ Deployment Environment

• Runtime: NodeJS v22.12.0
• Database: SQLite
• Filesystem used to store the database on: Windows on a SSD
• Number of monitors: 5

📝 Relevant log output

No response

[homelab-alpha]

@Ionys320, Sorry, I missed that you had created a new issue for this. Let's keep the conversation here regarding this bug.

Can you show it in a screencast what you mean? Because what you're saying, I can't recreate.

[homelab-alpha]

Okay, this is strange. When I manually add the code and use Uptime-Kuma in Dev mode, this bug doesn't occur. However, when I run the following command:

docker run --rm -it -p 3000:3000 -p 3001:3001 --pull always -e 'UPTIME_KUMA_GH_REPO=Ionys320:feat-search-with-url' louislam/uptime-kuma:pr-test2

I encounter this bug. Is this the same for you, @Ionys320?

[Ionys320]

@homelab-alpha, yes indeed, I changed my issue on purpose. This issue is related to the PR because the PR extends the used fields.

There is two possibilities to fix this:

• Edit the PR to use monitor.type in order to use a field or not
• Set to NULL/empty strings unused fields for a monitor at the save time (my favorite one).

[homelab-alpha]

@Ionys320, I can now also reproduce the bug in Uptime Kuma's Dev mode. If you delete the date folder where the db is stored and then start Dev mode with npm run dev, you will need to reconfigure Uptime Kuma. You will then encounter the bug you described.

As for the option 'Set to NULL/empty strings unused fields for a monitor at the save time (my favorite option)', I think this is a good fix.

[homelab-alpha]

@Ionys320, here is a revision of the code with a regular expression that escapes special characters. I have also updated the comments. However, the bug still exists in this revision unless you remove safeRegexTest(monitor.url) from the code; doing so resolves the issue. It's up to you to decide whether you agree with this. If you remove safeRegexTest(monitor.url) from the code, everything works as expected according to my testing.

Could you verify this for me.

// Filter monitors based on the search text
// Matches monitor name, URL, hostname, DNS resolve server, or tag names and values
let searchTextMatch = true;
if (this.searchText !== "") {
    try {
        // Escape special characters for use in the regular expression
        const escapeRegExp = (string) => {
            return string.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
        };

        // Create a case-insensitive regex for the search text
        const escapedSearchText = escapeRegExp(this.searchText);
        const regex = new RegExp(escapedSearchText, "i");

        // Test the regex against strings safely
        const safeRegexTest = (str) => str && regex.test(str);

        // Check if any relevant fields match the search text
        searchTextMatch =
            regex.test(monitor.name) ||
            safeRegexTest(monitor.url) ||
            safeRegexTest(monitor.hostname) ||
            safeRegexTest(monitor.dns_resolve_server) ||
            monitor.tags.some(tag => regex.test(tag.name) || safeRegexTest(tag.value));
    } catch (e) {
        // Log error if the regex pattern is invalid
        console.error("Invalid regex pattern:", e);
        searchTextMatch = false;
    }
}

[Ionys320]

Hi @homelab-alpha!
Indeed, it seems to work, but escaping the regex expression for each monitor cost a lot for execution time, and it seems we approved in #5438 the searchbar will be regex friendly.

Regarding ignoring monitor.url, it's not matching the PR anymore (and the problem would be similar with other fields, so the PR would be useless).

Therefore, I keep my issue opened to maybe approve the clean of unused fields before saving.

[homelab-alpha]

@Ionys320, It doesn't seem like a big problem to wait 0.1 seconds longer for escaping the regex expression, as this will cause fewer issues in the long run.

As for ignoring monitor.url, it seems like a good solution to me, as all functions are working now anyway, or am I mistaken?

[Ionys320]

Yeah indeed for the regex escape, I'll change the PR according to.

Regarding ignoring monitor.url, here is the issue: a monitor can't be found using its URL

Code:

searchTextMatch =
        regex.test(monitor.name) ||
        safeRegexTest(monitor.hostname) ||
        safeRegexTest(monitor.dns_resolve_server) ||
        monitor.tags.some(tag => regex.test(tag.name) || safeRegexTest(tag.value));

[homelab-alpha]

@Ionys320, my mistake. I misread and misunderstood your message.

You can apply the suggestion you made earlier to fix the bug:

• Set unused fields for a monitor to NULL or empty strings during save.

[homelab-alpha]

@Ionys320, update: okay, our issue is not a real bug, but rather a misconfiguration in Uptime Kuma itself.

When you add a new monitor, for example:

• Monitor Type: https
• Friendly Name: Test ONE
• URL: https://google.fr

And then change the monitor type to:

• Monitor Type: Ping
• Friendly Name: Test ONE
• URL: cloudflare.com

And you save the configuration, two different URLs are stored in the database:

1. The URL https://google.fr is used by the monitor type https.
2. The hostname cloudflare.com is used by the monitor type Ping.

Now, if we search for .fr using a regular expression, both monitor.url and monitor.hostname are searched, and we get the same result (in this case Test ONE) because the data in the database is stored like this.

Conclusion: It doesn't matter which monitor type you switch to, the fields you have modified are not removed. In this case, the URL from the initial input is retained, even when a new Hostname is requested. See the screenshot of the database.

I will be offline for the next few days due to the holidays. I wishing you and your loved ones a Merry Christmas 🎄✨🎁

screenshot (click to expand)

[homelab-alpha]

@Ionys320, if we make the following change in src/pages/EditMonitor.vue line 1722:

NOTE: I concluded that I need to do additional testing for the rest that rely on this.monitor.hostname.

From

if (this.monitor.url) {
    this.monitor.url = this.monitor.url.trim();
}

To

if (this.monitor?.hostname) {
    this.monitor.url = "https://";
    this.monitor.hostname = this.monitor.hostname.trim();
}

This will resolve the issue, as shown in the video.

video (click to expand)

screencast_from_2024_12_26_10_13_55.mp4

screenshot db (click to expand)

[homelab-alpha]

@Ionys320, I have completed my tests, and this is the correct solution in my opinion. I have checked all monitor types. And it possible to provide a default value for for monitor.url in this.monitor?.hostname. The default value is the same as what Uptime Kuma normally uses, namely https://.

My question to you now is whether you agree with this solution, and if you would be willing to test it to verify the solution?

The list of v-models is incomplete, but to check which monitor types use monitor.hostname and monitor.url, is complete.

monitore_type_list (click to expand)

Configuratie per Monitor Type

monitor.type === 'docker'

• v-model="monitor.docker_container"
• v-model="monitor.docker_host"

monitor.type === 'dns'

• v-model="monitor.dns_resolve_server"
• v-model="monitor.dns_resolve_type"
• v-model="monitor.hostname"

monitor.type === 'gamedig'

• v-model="monitor.game"
• v-model="monitor.gamedigGivenPortOnly"

monitor.type === 'group'

Geen extra configuratie

monitor.type === 'grpc-keyword'

• v-model="monitor.grpcUrl"
• v-model="monitor.grpcEnableTls"
• v-model="monitor.grpcServiceName"
• v-model="monitor.grpcMethod"
• v-model="monitor.grpcProtobuf"
• v-model="monitor.grpcBody"
• v-model="monitor.grpcMetadata"

monitor.type === 'http'

• v-model="monitor.url"

monitor.type === 'json-query'

• v-model="monitor.jsonPath"
• v-model="monitor.jsonPathOperator"
• v-model="monitor.url"

monitor.type === 'keyword'

• v-model="monitor.url"
• v-model="monitor.invertKeyword"
• v-model="monitor.keyword"

monitor.type === 'kafka-producer'

• v-model="monitor.kafkaProducerAllowAutoTopicCreation"
• v-model="monitor.kafkaProducerBrokers"
• v-model="monitor.kafkaProducerMessage"
• v-model="monitor.kafkaProducerSaslOptions"
  • v-model="monitor.kafkaProducerSaslOptions.accessKeyId"
  • v-model="monitor.kafkaProducerSaslOptions.authorizationIdentity"
  • v-model="monitor.kafkaProducerSaslOptions.mechanism"
  • v-model="monitor.kafkaProducerSaslOptions.password"
  • v-model="monitor.kafkaProducerSaslOptions.secretAccessKey"
  • v-model="monitor.kafkaProducerSaslOptions.sessionToken"
  • v-model="monitor.kafkaProducerSaslOptions.username"
• v-model="monitor.kafkaProducerSsl"
• v-model="monitor.kafkaProducerTopic"

monitor.type === 'mongodb'

• v-model="monitor.databaseConnectionString"
• v-model="monitor.databaseQuery"

monitor.type === 'mqtt'

• v-model="monitor.hostname"
• v-model="monitor.mqttCheckType"
• v-model="monitor.mqttPassword"
• v-model="monitor.mqttSuccessMessage"
• v-model="monitor.mqttTopic"
• v-model="monitor.mqttUsername"

monitor.type === 'mysql'

• v-model="monitor.databaseConnectionString"
• v-model="monitor.databaseQuery"

monitor.type === 'ping'

• v-model="monitor.hostname"

monitor.type === 'postgres'

• v-model="monitor.databaseConnectionString"
• v-model="monitor.databaseQuery"

monitor.type === 'push'

• v-model="pushURL"

monitor.type === 'rabbitmq'

• v-model="monitor.rabbitmqNodes"
• v-model="monitor.rabbitmqPassword"
• v-model="monitor.rabbitmqUsername"

monitor.type === 'radius'

• v-model="monitor.hostname"
• v-model="monitor.radiusCalledStationId"
• v-model="monitor.radiusCallingStationId"
• v-model="monitor.radiusPassword"
• v-model="monitor.radiusSecret"
• v-model="monitor.radiusUsername"

monitor.type === 'real-browser'

• v-model="monitor.remote_browser"
• v-model="monitor.url"

monitor.type === 'redis'

• v-model="monitor.databaseConnectionString"

monitor.type === 'snmp'

• v-model="monitor.hostname"
• v-model="monitor.port"
• v-model="monitor.snmpOid"
• v-model="monitor.snmpVersion"

monitor.type === 'steam'

• v-model="monitor.hostname"

monitor.type === 'tailscale-ping'

• v-model="monitor.hostname"
• v-model="monitor.port"

monitor.type === 'port'

• v-model="monitor.hostname"
• v-model="monitor.port"

monitor.type === 'sqlserver'

• v-model="monitor.databaseConnectionString"
• v-model="monitor.databaseQuery"

[Ionys320]

Hi @homelab-alpha, hope you have a nice Christmas! Regarding your work, would you mind doing a PR to simplify the tests? I'm not the one that will approve it anyway, but I can still test it.

[homelab-alpha]

@Ionys320, Thanks for asking, I had a great Christmas. I hope it was the same for you. To be honest, I have never done a PR before, only for myself for testing, and I make enough mistakes with that already. So, if you don't mind, you can make the change yourself in src/pages/EditMonitor.vue, line 1722:

From:

if (this.monitor.url) {
    this.monitor.url = this.monitor.url.trim();
}

To:

if (this.monitor?.hostname) {
    this.monitor.url = "https://";
    this.monitor.hostname = this.monitor.hostname.trim();
}

And your PR is still in draft, so you can make the change directly in your PR. I'm sorry, I'm still learning

[homelab-alpha]

@Ionys320, I created a PR with the change of searchText from stringbase to regexbase, plus the fix for this issue.

docker run --rm -it -p 3000:3000 -p 3001:3001 --pull always -e 'UPTIME_KUMA_GH_REPO=homelab-alpha:feat-searchText-from-stringbase-to-regexbase' louislam/uptime-kuma:pr-test2

[homelab-alpha]

@Ionys320, it's not cool to give a thumbs down. You yourself said, I quote: "Would you mind doing a PR to simplify the tests?" As I mentioned before, I had never created a PR.

I figured out how to create a proper PR, and now this isn't the intention?! What's the problem? If you wanted something different, you should have put more effort into clarifying your intentions.

[Ionys320]

Hi @homelab-alpha!
Yes sorry, I put the reaction to reminds me the PR wasn't fixing the issue for all the tests.

I think I'll do another issue instead of keeping this one, the problem is larger than just having noise, it's also about removing irrelevant data from the DB.

[homelab-alpha]

@Ionys320, The problem lies in the "Add New Monitor" menu, as the URL is processed in various ways (such as monitor.url, monitor.hostname, monitor.grpcUrl, etc.). The solution is that the "Add New Monitor" menu needs to be completely rewritten for better compatibility.