Re: svn commit: r39219 - trunk/subversion/libsvn_subr

On Thu, Sep 10, 2009 at 7:32 AM, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> Author: cmpilato
> Date: Thu Sep 10 07:32:11 2009
> New Revision: 39219
>
> Log:
> Finish issue #3479 - mime.types extension matching case sensitive
>
> When I added the feature that allows you to specify a MIME mapping
> file in your runtime configuration (for use when doing automatic
> property setting on 'add' and 'import'), I failed to recall that those
> mappings are meant to be interpreted case-insensitively.  This commit
> remedies that.
>
> * subversion/libsvn_subr/io.c
>  (file_ext_tolower): New function, copied wholesale from
>    libsvn_subr/config.c:make_hash_key().
>  (svn_io_parse_mimetypes_file): Use file_ext_tolower() to ensure that
>    the keys in the MIME mappings hash are lower-cased.
>  (svn_io_detect_mimetype2): Use file_ext_tolower() to normalize the
>    extension of the filename we're detecting MIME types for to
>    lowercase so it has a better chance of matching items in the MIME
>    mapping hash.  While here, avoid testing for file existence until
>    *after* we fail at name-based MIME association.
>
> Modified:
>   trunk/subversion/libsvn_subr/io.c
>
> Modified: trunk/subversion/libsvn_subr/io.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/io.c?pathrev=39219&r1=39218&r2=39219
> ==============================================================================
> --- trunk/subversion/libsvn_subr/io.c   Thu Sep 10 06:54:20 2009        (r39218)
> +++ trunk/subversion/libsvn_subr/io.c   Thu Sep 10 07:32:11 2009        (r39219)
> @@ -2564,6 +2564,18 @@ svn_io_run_diff3_3(int *exitcode,
>   return SVN_NO_ERROR;
>  }
>
> +
> +/* Canonicalize a string for hashing.  Modifies KEY in place. */
> +static APR_INLINE char *
> +fileext_tolower(char *key)
> +{
> +  register char *p;
> +  for (p = key; *p != 0; ++p)
> +    *p = apr_tolower(*p);
> +  return key;
> +}
> +
> +
>  svn_error_t *
>  svn_io_parse_mimetypes_file(apr_hash_t **type_map,
>                             const char *mimetypes_file,
> @@ -2612,6 +2624,7 @@ svn_io_parse_mimetypes_file(apr_hash_t *
>           for (i = 1; i < tokens->nelts; i++)
>             {
>               const char *ext = APR_ARRAY_IDX(tokens, i, const char *);
> +              fileext_tolower((char *)ext);
>               apr_hash_set(types, ext, APR_HASH_KEY_STRING, type);
>             }
>         }
> @@ -2653,13 +2666,6 @@ svn_io_detect_mimetype2(const char **mim
>   /* Default return value is NULL. */
>   *mimetype = NULL;
>
> -  /* See if this file even exists, and make sure it really is a file. */
> -  SVN_ERR(svn_io_check_path(file, &kind, pool));
> -  if (kind != svn_node_file)
> -    return svn_error_createf(SVN_ERR_BAD_FILENAME, NULL,
> -                             _("Can't detect MIME type of non-file '%s'"),
> -                             svn_dirent_local_style(file, pool));
> -
>   /* If there is a mimetype_map provided, we'll first try to look up
>      our file's extension in the map.  Failing that, we'll run the
>      heuristic. */
> @@ -2667,6 +2673,7 @@ svn_io_detect_mimetype2(const char **mim
>     {
>       const char *type_from_map, *path_ext;
>       svn_path_splitext(NULL, &path_ext, file, pool);
> +      fileext_tolower((char *)path_ext);
>       if ((type_from_map = apr_hash_get(mimetype_map, path_ext,
>                                         APR_HASH_KEY_STRING)))
>         {
> @@ -2675,6 +2682,13 @@ svn_io_detect_mimetype2(const char **mim
>         }
>     }
>
> +  /* See if this file even exists, and make sure it really is a file. */
> +  SVN_ERR(svn_io_check_path(file, &kind, pool));
> +  if (kind != svn_node_file)
> +    return svn_error_createf(SVN_ERR_BAD_FILENAME, NULL,
> +                             _("Can't detect MIME type of non-file '%s'"),
> +                             svn_dirent_local_style(file, pool));
> +
>   SVN_ERR(svn_io_file_open(&fh, file, APR_READ, 0, pool));
>
>   /* Read a block of data from FILE. */
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=2393310
>

Hi,

This commit started a Ruby bindings failure. Here is the relevant
scrap of test code
(I hope this is clear enough to anyone who is not familiar with Ruby,
if not please ask):

  def test_mime_type_detect_with_type_map
    type_map = {
      "html" => "text/html",
      "htm" => "text/html",
      "png" => "image/png",
    }

    nonexistent_html_file = File.join(@tmp_path, "nonexistent.html")
    assert_raises(Svn::Error::BadFilename) do
      Svn::Core::MimeType.detect(nonexistent_html_file)
    end
    assert_raises(Svn::Error::BadFilename) do
      Svn::Core::MimeType.detect(nonexistent_html_file, type_map) ###
    end

The second call to Svn::Core::MimeType.detect is no longer raising (marked
### above). I wonder if changing the placement of the block that checks for
existence is the culprit? Or if this was an expected change, I will of course
update the test.

Thanks,

--
Joe
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2396912