Improve behaviour with too long commands/filenames.

lib/gen_magic/server.ex

@@ -283,7 +283,7 @@ defmodule GenMagic.Server do
     20 => :enotdir,
     12 => :enomem,
     24 => :emfile,
-    36 => :enametoolong,
+    36 => :enametoolong
   }
   @errno Map.keys(@errnos)
 
@@ -292,6 +292,7 @@ defmodule GenMagic.Server do
       {:ok, {mime_type, encoding, content}} -> {:ok, Result.build(mime_type, encoding, content)}
       {:error, {errno, _}} when errno in @errno -> {:error, @errnos[errno]}
       {:error, {errno, string}} -> {:error, "#{errno}: #{string}"}
+      {:error, _} = error -> error
     end
   end
 

src/apprentice.c

@@ -60,6 +60,11 @@
 #define ERROR_BAD_TERM 4
 #define ERROR_EI 5
 
+// We use a bigger than possible valid command length (around 4111 bytes) to
+// allow more precise errors when using too long paths.
+#define COMMAND_LEN 8000
+#define COMMAND_BUFFER_SIZE COMMAND_LEN + 1
+
 #define MAGIC_FLAGS_COMMON (MAGIC_CHECK | MAGIC_ERROR)
 magic_t magic_setup(int flags);
 
@@ -70,14 +75,14 @@ void setup_options(int argc, char **argv);
 void setup_options_file(char *optarg);
 void setup_options_default();
 void setup_system();
-int process_command(byte *buf);
+int process_command(uint16_t len, byte *buf);
-void process_line(char *line);
 void process_file(char *path, ei_x_buff *result);
 void process_bytes(char *bytes, int size, ei_x_buff *result);
 size_t read_cmd(byte *buf);
 size_t write_cmd(byte *buf, size_t len);
 void error(ei_x_buff *result, const char *error);
 void handle_magic_error(magic_t handle, int errn, ei_x_buff *result);
+void fdseek(uint16_t count);
 
 struct magic_file {
   struct magic_file *prev;
@@ -103,29 +108,35 @@ int main(int argc, char **argv) {
   if (ei_x_free(&ok_buf) != 0)
     exit(ERROR_EI);
 
-  byte buf[4112];
+  byte buf[COMMAND_BUFFER_SIZE];
-  while (read_cmd(buf) > 0) {
+  uint16_t len;
-    process_command(buf);
+  while ((len = read_cmd(buf)) > 0) {
+    process_command(len, buf);
   }
 
   return 255;
 }
 
-int process_command(byte *buf) {
+int process_command(uint16_t len, byte *buf) {
   ei_x_buff result;
   char atom[128];
   int index, version, arity, termtype, termsize;
   index = 0;
 
-  if (ei_decode_version(buf, &index, &version) != 0) {
-    exit(ERROR_BAD_TERM);
-  }
-
   // Initialize result
   if (ei_x_new_with_version(&result) || ei_x_encode_tuple_header(&result, 2)) {
     exit(ERROR_EI);
   }
 
+  if (len >= COMMAND_LEN) {
+    error(&result, "badarg");
+    return 1;
+  }
+
+  if (ei_decode_version(buf, &index, &version) != 0) {
+    exit(ERROR_BAD_TERM);
+  }
+
   if (ei_decode_tuple_header(buf, &index, &arity) != 0) {
     error(&result, "badarg");
     return 1;
@@ -145,11 +156,16 @@ int process_command(byte *buf) {
     char path[4097];
     ei_get_type(buf, &index, &termtype, &termsize);
 
-    if (termtype == ERL_BINARY_EXT && termsize < 4096) {
+    if (termtype == ERL_BINARY_EXT) {
+      if (termsize < 4096) {
         long bin_length;
         ei_decode_binary(buf, &index, path, &bin_length);
         path[termsize] = '\0';
         process_file(path, &result);
+      } else {
+        error(&result, "enametoolong");
+        return 1;
+      }
     } else {
       error(&result, "badarg");
       return 1;
@@ -176,6 +192,7 @@ int process_command(byte *buf) {
     return 1;
   }
 
+  // Empty the buffer.
   write_cmd(result.buff, result.index);
 
   if (ei_x_free(&result) != 0) {
@@ -386,9 +403,15 @@ size_t read_cmd(byte *buf) {
   }
   uint16_t len16 = *(uint16_t *)buf;
   len16 = ntohs(len16);
-  if (len16 > 4111) {
+
-    exit(ERROR_BAD_TERM);
+  // Buffer isn't large enough: just return possible len, without reading.
+  // Up to the caller of verifying the size again and return an error.
+  // buf left unchanged.
+  if (len16 > COMMAND_LEN) {
+    fdseek(len16);
+    return len16;
   }
+
   return read_exact(buf, len16);
 }
 
@@ -412,3 +435,11 @@ void error(ei_x_buff *result, const char *error) {
   if (ei_x_free(result) != 0)
     exit(ERROR_EI);
 }
+
+void fdseek(uint16_t count) {
+  int i = 0;
+  while (i < count) {
+    getchar();
+    i += 1;
+  }
+}

test/gen_magic/apprentice_test.exs

@@ -1,6 +1,8 @@
 defmodule GenMagic.ApprenticeTest do
   use GenMagic.MagicCase
 
+  @tmp_path "/tmp/testgenmagicx"
+
   test "sends ready" do
     port = Port.open(GenMagic.Config.get_port_name(), GenMagic.Config.get_port_options([]))
     assert_ready(port)
@@ -85,19 +87,39 @@ defmodule GenMagic.ApprenticeTest do
     end
 
     test "works with big file path", %{port: port} do
-      file = too_big() <> "/a"
+      # Test with longest valid path.
-      File.mkdir_p!(too_big())
+      {dir, bigfile} = too_big(@tmp_path, "/a")
-      File.touch!(file)
+      File.mkdir_p!(dir)
-      on_exit(fn -> File.rm_rf!("/tmp/testmagicex/") end)
+      File.touch!(bigfile)
-      send(port, {self(), {:command, :erlang.term_to_binary({:file, file})}})
+      on_exit(fn -> File.rm_rf!(@tmp_path) end)
+      send(port, {self(), {:command, :erlang.term_to_binary({:file, bigfile})}})
       assert_receive {^port, {:data, data}}
       assert {:ok, _} = :erlang.binary_to_term(data)
       refute_receive _
-      file = too_big() <> "/aaaaaaaaaa"
+
+      # This path should be long enough for buffers, but larger than a valid path name. Magic will return an errno 36.
+      file = @tmp_path <> String.duplicate("a", 256)
+      send(port, {self(), {:command, :erlang.term_to_binary({:file, file})}})
+      assert_receive {^port, {:data, data}}
+      assert {:error, {36, _}} = :erlang.binary_to_term(data)
+      refute_receive _
+      # Theses filename should be too big for the path buffer.
+      file = bigfile <> "aaaaaaaaaa"
+      send(port, {self(), {:command, :erlang.term_to_binary({:file, file})}})
+      assert_receive {^port, {:data, data}}
+      assert {:error, :enametoolong} = :erlang.binary_to_term(data)
+      refute_receive _
+      # This call should be larger than the COMMAND_BUFFER_SIZE. Ensure nothing bad happens!
+      file = String.duplicate(bigfile, 4)
       send(port, {self(), {:command, :erlang.term_to_binary({:file, file})}})
       assert_receive {^port, {:data, data}}
       assert {:error, :badarg} = :erlang.binary_to_term(data)
       refute_receive _
+      # We re-run a valid call to ensure the buffer/... haven't been corrupted in port land.
+      send(port, {self(), {:command, :erlang.term_to_binary({:file, bigfile})}})
+      assert_receive {^port, {:data, data}}
+      assert {:ok, _} = :erlang.binary_to_term(data)
+      refute_receive _
     end
   end
 
@@ -106,7 +128,20 @@ defmodule GenMagic.ApprenticeTest do
     assert :ready == :erlang.binary_to_term(data)
   end
 
-  def too_big do
+  def too_big(path, filename, limit \\ 4095) do
-    "/tmp/testmagicex/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+    last_len = byte_size(filename)
+    path_len = byte_size(path)
+    needed = limit - (last_len + path_len)
+    extra = make_too_big(needed, "")
+    {path <> extra, path <> extra <> filename}
+  end
+
+  def make_too_big(needed, acc) when needed <= 255 do
+    acc <> "/" <> String.duplicate("a", needed - 1)
+  end
+
+  def make_too_big(needed, acc) do
+    acc = acc <> "/" <> String.duplicate("a", 254)
+    make_too_big(needed - 255, acc)
   end
 end