15
\$\begingroup\$

I just developed a URL shortener.

index.php:

<?
    $sql_host = "...";
    $sql_db = "...";
    $sql_user = "...";
    $sql_pass = "...";

    $conn_error = "Could not connect to database";

    $con = mysqli_connect($sql_host, $sql_user, $sql_pass, $sql_db) or die($conn_error);

    $hash = htmlspecialchars($_GET["l"]);
    $hash = mysqli_real_escape_string($con, $hash);

    if (!(empty($hash))) {
        $query = "SELECT * FROM `links` WHERE hash = '$hash'";
        $row = mysqli_query($con, $query) or die($conn_error);
        if (mysqli_num_rows($row)) {
            $_ROW = mysqli_fetch_assoc($row);
            $url = $_ROW["link"];
            header("Location: $url");
            exit;
        } else {
            die("link not available");
        }
    }
?>
<html>
    <head>
        <title>gus URL Shortener</title>
    </head>
    <body>
        Type the link to be shortened:<br>
        <form action="short.php" method="post">
        <input type="text" name="url"><input type="submit" value="Shorten">
        </form>
    </body>
</html> 

The form redirects to short.php where the link is generated:

<html>
    <?
        $sql_host = "...";
        $sql_db = "...";
        $sql_user = "...";
        $sql_pass = "...";

        $conn_error = "Could not connect to database";

        $con = mysqli_connect($sql_host, $sql_user, $sql_pass, $sql_db) or die($conn_error);

        $url = $_POST["url"];
        $url = mysqli_real_escape_string($con, $url);
        $hash = hash("crc32", "$url");

        if (!(empty($url))) {
            if ((substr($url, 0, 7) == "http://") or (substr($url, 0, 8) == "https://")) {
                $query = "INSERT INTO `links` (hash, link)
                          VALUES ('$hash', '$url')";
                mysqli_query($con, $query) or die($conn_error);
            } else {
                die("Your link needs to start with http:// or https://");
            }
        }
    ?>
    <head>
        <title>gus URL Shortener</title>
    </head>
    <body>
        Your Link:
        <? echo "<a href='http://gus.netii.net/?l=$hash'>gus.netii.net/?l=$hash</a>";?>
    </body>
</html>

Sorry it's not commented but I think it's pretty straightforward. I've been thinking how could I do it so the link doesn't have the "?l=..." like other shorteners have.

UPDATE

Now gus.netii.net is up and running safely, like @CodeX suggested, also without the "?l=" thing after the url to GET with php ! Thanks to all and share it please.

\$\endgroup\$
8
  • 1
    \$\begingroup\$ url rewrite might work with.htaccess \$\endgroup\$
    – CodeX
    Jul 21, 2014 at 15:17
  • \$\begingroup\$ BTW is your hash meant to be a random hash? \$\endgroup\$
    – CodeX
    Jul 21, 2014 at 16:46
  • \$\begingroup\$ yep, random but unique \$\endgroup\$ Jul 21, 2014 at 16:47
  • 1
    \$\begingroup\$ google.com for example will always be db85f073 using crc32 \$\endgroup\$
    – CodeX
    Jul 21, 2014 at 16:49
  • 1
    \$\begingroup\$ This is also good reading - blog.codinghorror.com/url-shortening-hashes-in-practice \$\endgroup\$
    – CodeX
    Jul 21, 2014 at 16:56

2 Answers 2

11
\$\begingroup\$

Ok, so the main problem with your code is that it is vulnerable to SQL injection you can fix that by using prepared statements - http://php.net/manual/en/mysqli.quickstart.prepared-statements.php

If you want to remove the ?l= you can look into Mod Rewrite - http://www.sitepoint.com/guide-url-rewriting/ and have the URL generate as gus.netii.net/$hash

I took the MySQLi Code straight from PHP.net to use in this example

index.php

$sql_host = "";
$sql_db = "";
$sql_user = "";
$sql_pass = "";

$mysqli = new mysqli($sql_host, $sql_user, $sql_pass, $sql_db);
if ($mysqli->connect_errno) {
    echo "Failed to connect to MySQL: (" . $mysqli->connect_errno . ") " . $mysqli->connect_error;
}

if(isset($_GET['l'])) {
    $hash = htmlspecialchars($_GET["l"]);
}

if (!empty($hash)) {
    if (!($stmt = $mysqli->prepare("SELECT link FROM links WHERE hash = ?"))) {
            echo "Prepare failed: (" . $mysqli->errno . ") " . $mysqli->error;
    }

    if (!$stmt->bind_param("s", $hash)) {
        echo "Binding parameters failed: (" . $stmt->errno . ") " . $stmt->error;
    }

    if (!$stmt->execute()) {
        echo "Execute failed: (" . $stmt->errno . ") " . $stmt->error;
    }

    if ($stmt->execute()) {
        $stmt->bind_result($url);
        $stmt->fetch();
        header("Location: $url");
    } else {
        echo "link not available";
    }
}

short.php

$sql_host = "";
$sql_db = "";
$sql_user = "";
$sql_pass = "";

$mysqli = new mysqli($sql_host, $sql_user, $sql_pass, $sql_db);
if ($mysqli->connect_errno) {
    echo "Failed to connect to MySQL: (" . $mysqli->connect_errno . ") " . $mysqli->connect_error;
}

$url = $_POST["url"];
$hash = hash("crc32", "$url");

if (!empty($url)) {
    if ((substr($url, 0, 7) == "http://") or (substr($url, 0, 8) == "https://")) {
        if (!($stmt = $mysqli->prepare("INSERT INTO links(hash, link) VALUES (?,?)"))) {
            echo "Prepare failed: (" . $mysqli->errno . ") " . $mysqli->error;
        }
        if (!$stmt->bind_param("ss", $hash, $url)) {
            echo "Binding parameters failed: (" . $stmt->errno . ") " . $stmt->error;
        }
        if (!$stmt->execute()) {
            echo "Execute failed: (" . $stmt->errno . ") " . $stmt->error;
        }
        echo "<a href='http://gus.netii.net/?l=".$hash."'>gus.netii.net/?l=".$hash."</a>";
    } 
    else {
        echo "Your link needs to start with http:// or https://";
    }
}
\$\endgroup\$
2
  • \$\begingroup\$ what $stmt stands for ? \$\endgroup\$ Jul 21, 2014 at 22:55
  • 5
    \$\begingroup\$ @matheussilvapb $stmt usually is a shorthand term for "statement", which is meant to represent a prepared statement. \$\endgroup\$
    – Alex L
    Jul 21, 2014 at 23:21
6
\$\begingroup\$

Apart from fixing the SQL injection with prepared statements as shown by CodeX you should/could do

  • Move your SQL data and possibly also the connecting to the database into a config.php
  • You didn't specify the table layout so make sure to properly set the unique property for fields
  • Consider using a different database more suited for the task. SQL databases are for relational data. You don't really have that so you could use a NoSQL system such as Redis for speed improvements.
  • Do an more through checking whether it is a valid url. E.g. only a limited character set and no succeeding periods in the hostname, ... You can use filter_var() for this
  • Not use a hash but an incrementally rising ID to save URL space. Easily doable in redis (INCR), and to shorten the length of the ID it could be represented in baseX.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.