[PR #33] [MERGED] Handle special characters #40

Closed
opened 2026-02-27 23:20:11 +03:00 by kerem · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/spatie/dnsrecords.io/pull/33
Author: @brendt
Created: 11/1/2017
Status: Merged
Merged: 11/1/2017
Merged by: @freekmurze

Base: masterHead: handle-special-characters


📝 Commits (3)

  • 94f2f35 Add POST route on / so that special characters like ? and . don't trigger a bad request.
  • 64ee0fa Add domain lookup sanitizer middleware
  • 48c9364 Add domain to page title

📊 Changes

11 files changed (+123 additions, -51 deletions)

View changed files

📝 app/Http/Controllers/HomeController.php (+5 -1)
📝 app/Http/Kernel.php (+1 -0)
app/Http/Middleware/SanitizeDnsLookup.php (+32 -0)
📝 app/Services/Commands/Commands/DnsLookup.php (+5 -4)
📝 resources/assets/js/app.js (+2 -5)
📝 resources/views/layout/master.blade.php (+1 -1)
📝 routes/web.php (+11 -2)
tests/Feature/DnsLookupTest.php (+56 -0)
tests/Feature/ExampleTest.php (+0 -20)
📝 tests/TestCase.php (+10 -0)
tests/Unit/ExampleTest.php (+0 -18)

📄 Description

This PR adds a few things:

  • First of all it fixes the 500 crashes reported in #28. This was caused by some special characters which don't register as a {command} in Laravel's router: ., .. and ?. Fixing this issue is done by allowing POST requests on /. It won't show any results though, which for . would be a nice to have. I will work on a fix for that one soon.
  • Tests for the DNS lookup are added, addressing #29. There's still room for improvement though, as all commands should be tested.
  • All URLs are sanitized server-side. This means that lookups like http:// or containing a path in the URL will be transformed to just the host, and you'll be redirected to that URL before showing results. By doing so, the hardcoded .replace in JavaScript has been removed. Sanitizing this URL and redirecting is done in a middleware, and uses the same DnsRecordsRetriever as the lookup command itself.

Todo:

  • Make . lookup work.
  • Add more tests.

Though a review is already welcome.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/spatie/dnsrecords.io/pull/33 **Author:** [@brendt](https://github.com/brendt) **Created:** 11/1/2017 **Status:** ✅ Merged **Merged:** 11/1/2017 **Merged by:** [@freekmurze](https://github.com/freekmurze) **Base:** `master` ← **Head:** `handle-special-characters` --- ### 📝 Commits (3) - [`94f2f35`](https://github.com/spatie/dnsrecords.io/commit/94f2f35ce51c43b92dbe4001f0effcdcfe40dff2) Add POST route on / so that special characters like ? and . don't trigger a bad request. - [`64ee0fa`](https://github.com/spatie/dnsrecords.io/commit/64ee0fa502faca344c50b372b9d7e91af78bd50b) Add domain lookup sanitizer middleware - [`48c9364`](https://github.com/spatie/dnsrecords.io/commit/48c9364e5e58c7e712acb3bfb2d97b485b44cec5) Add domain to page title ### 📊 Changes **11 files changed** (+123 additions, -51 deletions) <details> <summary>View changed files</summary> 📝 `app/Http/Controllers/HomeController.php` (+5 -1) 📝 `app/Http/Kernel.php` (+1 -0) ➕ `app/Http/Middleware/SanitizeDnsLookup.php` (+32 -0) 📝 `app/Services/Commands/Commands/DnsLookup.php` (+5 -4) 📝 `resources/assets/js/app.js` (+2 -5) 📝 `resources/views/layout/master.blade.php` (+1 -1) 📝 `routes/web.php` (+11 -2) ➕ `tests/Feature/DnsLookupTest.php` (+56 -0) ➖ `tests/Feature/ExampleTest.php` (+0 -20) 📝 `tests/TestCase.php` (+10 -0) ➖ `tests/Unit/ExampleTest.php` (+0 -18) </details> ### 📄 Description This PR adds a few things: - First of all it fixes the 500 crashes reported in #28. This was caused by some special characters which don't register as a `{command}` in Laravel's router: `.`, `..` and `?`. Fixing this issue is done by allowing POST requests on `/`. It won't show any results though, which for `.` would be a nice to have. I will work on a fix for that one soon. - Tests for the DNS lookup are added, addressing #29. There's still room for improvement though, as all commands should be tested. - All URLs are sanitized server-side. This means that lookups like `http://` or containing a path in the URL will be transformed to just the host, and you'll be redirected to that URL before showing results. By doing so, the hardcoded `.replace` in JavaScript has been removed. Sanitizing this URL and redirecting is done in a middleware, and uses the same `DnsRecordsRetriever ` as the lookup command itself. Todo: - Make `.` lookup work. - Add more tests. Though a review is already welcome. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
kerem 2026-02-27 23:20:11 +03:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/dnsrecords.io#40
No description provided.