David, just a quick ping: Apologies not to have reviewed this yet,
it's only due to the fire-fighting re issue #1000. Your patch is not
forgotten (nor are the others that have been posted during the last
few days). We're close to done w/ 1000, and will be able to resume
regularly scheduled, uh, programming :-).
If anyone else has a chance to review this sooner, that would be
great of course.
-K
David Kimdon <dwhedon@debian.org> writes:
> Here is a patch to fix issue 920. We abort the eol-style propset if
> the svn:mime-type property is binary or if the file has inconsistent
> newlines. Any comments are appreciated. I also added some test
> cases, see the end of the patch. If you can think of others that this
> doesn't catch and should that would be especially helpful.
>
> -David
>
> Fix issue 920:
>
> * subversion/libsvn_wc/props.c :
> (svn_wc_prop_set): Check files before setting a svn:eol-style property.
> If the file is binary or has inconsistent eol-style then don't set the
> property.
> (validate_eol_prop_against_file): New function.
>
> * subversion/tests/clients/cmdline/prop_tests.py :
> (inappropriate_props): Add tests to make sure svn:eol-style cannot be set on
> inappropriate files.
>
>
>
> Index: subversion/libsvn_wc/props.c
> ===================================================================
> --- subversion/libsvn_wc/props.c (revision 3814)
> +++ subversion/libsvn_wc/props.c (working copy)
> @@ -1027,6 +1027,155 @@
> }
> }
>
> +enum svn_eol_type {
> + svn_eol_type_lf, /* '\n' Unix */
> + svn_eol_type_cr, /* '\r' Macintosh */
> + svn_eol_type_crlf, /* '\r\n' DOS */
> + svn_eol_type_none, /* unknown, or no eol chars present */
> +} svn_eol_type;
> +
> +static const char *svn_eol_strs[3] = {
> + "\n",
> + "\r",
> + "\r\n",
> +};
> +
> +/* Determine whether it is legal to set svn:eol-style property
> + on PATH. If the file is binary or has inconsistent eol-style
> + then svn:eol-style should not be set. */
> +static svn_error_t *
> +validate_eol_prop_against_file (const char *path,
> + apr_pool_t *pool)
> +{
> + char buffer[BUFSIZ];
> + apr_file_t *fp;
> + apr_size_t len;
> + int n;
> + int eol_type = svn_eol_type_none;
> + apr_status_t status;
> + apr_off_t offset;
> + const svn_string_t *mime_type;
> + int got_cr = 0;
> +
> + SVN_ERR (svn_wc_prop_get (&mime_type, SVN_PROP_MIME_TYPE, path, pool));
> +
> + if (mime_type && svn_mime_type_is_binary (mime_type->data))
> + return svn_error_createf (SVN_ERR_ILLEGAL_TARGET, 0, NULL,
> + "File `%s' has binary prop '" SVN_PROP_MIME_TYPE
> + " : %s'", path, mime_type->data);
> +
> + SVN_ERR (svn_io_file_open (&fp, path, APR_READ | APR_BINARY | APR_BUFFERED,
> + 0, pool));
> +
> + /* Detect preliminary eol style. Look for eol chars. The first eol
> + * chars we see determine the eol-style we will test for. */
> +
> + len = sizeof (buffer);
> + offset = 0;
> +
> + while (! (status = apr_file_read (fp, buffer, &len))
> + && eol_type == svn_eol_type_none)
> + {
> +
> + for (n = 0; n < len; n++)
> + {
> + char c = buffer[n];
> +
> + offset++;
> +
> + if (got_cr)
> + {
> + if (c == '\n')
> + eol_type = svn_eol_type_crlf;
> + else
> + eol_type = svn_eol_type_cr;
> + break;
> + }
> + else if (c == '\n')
> + {
> + eol_type = svn_eol_type_lf;
> + break;
> + }
> + else if (c == '\r')
> + {
> + got_cr = 1;
> + }
> + }
> + len = sizeof (buffer); /* for call to apr_file_read() */
> + }
> +
> +
> + if (status && ! APR_STATUS_IS_EOF (status))
> + {
> + apr_file_close (fp);
> + return svn_error_createf (status, 0, NULL,
> + "error reading file `%s'", path);
> + }
> +
> + if (eol_type == svn_eol_type_none)
> + {
> + /* No EOL chars present in the file. */
> + apr_file_close (fp);
> + return SVN_NO_ERROR;
> + }
> +
> + /* Verify that the eol style is consistent for the remainder
> + of the file. */
> +
> + if ((status = apr_file_seek (fp, APR_SET, &offset)))
> + {
> + apr_file_close (fp);
> + return svn_error_createf (status, 0, NULL,
> + "failed to seek file `%s'", path);
> + }
> + else
> + {
> + const char *eol_str = svn_eol_strs[eol_type];
> + int eol_str_len = strlen (eol_str);
> + int eol_index = 0;
> +
> + len = sizeof(buffer);
> +
> + while (! (status = apr_file_read (fp, buffer, &len)))
> + {
> + for (n = 0; n < len; n++)
> + {
> + char c = buffer[n];
> +
> + if (c == eol_str[eol_index])
> + {
> + if (eol_index == eol_str_len - 1)
> + eol_index = 0;
> + else
> + eol_index++;
> + }
> + else if ( c == '\n' || c == '\r' )
> + {
> + apr_file_close (fp);
> + return svn_error_createf (SVN_ERR_ILLEGAL_TARGET, 0, NULL,
> + "Inconsistent eol style in `%s'.",
> + path);
> + }
> + }
> + len = sizeof (buffer); /* for call to apr_file_read() */
> + }
> +
> + if (eol_index)
> + return svn_error_createf (SVN_ERR_ILLEGAL_TARGET, 0, NULL,
> + "Incomplete eol in `%s'.", path);
> + }
> +
> + apr_file_close (fp);
> +
> + if (! APR_STATUS_IS_EOF (status))
> + {
> + return svn_error_createf (status, 0, NULL,
> + "reading file `%s'", path);
> + }
> +
> + return SVN_NO_ERROR;
> +}
> +
>
> /* The special Subversion properties are not valid for all node kinds.
> Return an error if NAME is an invalid Subversion property for PATH which
> @@ -1103,7 +1252,12 @@
> property is allowed since older clients allowed (and other clients
> possibly still allow) setting it. */
> if (value)
> - SVN_ERR (validate_prop_against_node_kind (name, path, kind, pool));
> + {
> + SVN_ERR (validate_prop_against_node_kind (name, path, kind, pool));
> +
> + if (strcmp (name, SVN_PROP_EOL_STYLE) == 0)
> + SVN_ERR (validate_eol_prop_against_file (path, pool));
> + }
>
> if (kind == svn_node_file && strcmp (name, SVN_PROP_EXECUTABLE) == 0)
> {
> Index: subversion/tests/clients/cmdline/prop_tests.py
> ===================================================================
> --- subversion/tests/clients/cmdline/prop_tests.py (revision 3814)
> +++ subversion/tests/clients/cmdline/prop_tests.py (working copy)
> @@ -590,6 +590,53 @@
> if svntest.actions.run_and_verify_status(wc_dir, expected_status):
> return 1
>
> +# Issue #920. Don't allow setting of svn:eol-style on binary files or files
> +# with inconsistent eol stypes.
> +
> + path = os.path.join(wc_dir, 'binary')
> + svntest.main.file_append(path, "binary")
> + svntest.main.run_svn(None, 'add', path)
> +
> + svntest.main.run_svn(None, 'propset', 'svn:mime-type',
> + 'application/octet-stream', path)
> +
> + outlines,errlines = svntest.main.run_svn('Illegal target', 'propset',
> + 'svn:eol-style',
> + 'CRLF', path)
> + if not errlines:
> + return 1
> +
> + path = os.path.join(wc_dir, 'multi-eol')
> + svntest.main.file_append(path, "line1\rline2\n")
> + svntest.main.run_svn(None, 'add', path)
> +
> + outlines,errlines = svntest.main.run_svn('Illegal target', 'propset',
> + 'svn:eol-style',
> + 'LF', path)
> + if not errlines:
> + return 1
> +
> + path = os.path.join(wc_dir, 'backwards-eol')
> + svntest.main.file_append(path, "line1\n\r")
> + svntest.main.run_svn(None, 'add', path)
> +
> + outlines,errlines = svntest.main.run_svn('Illegal target', 'propset',
> + 'svn:eol-style',
> + 'native', path)
> + if not errlines:
> + return 1
> +
> + path = os.path.join(wc_dir, 'incomplete-eol')
> + svntest.main.file_append(path, "line1\r\n\r")
> + svntest.main.run_svn(None, 'add', path)
> +
> + outlines,errlines = svntest.main.run_svn('Illegal target', 'propset',
> + 'svn:eol-style',
> + 'CR', path)
> + if not errlines:
> + return 1
> +
> +
>
> #----------------------------------------------------------------------
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Nov 21 05:54:55 2002