Code review time.  :-)

On Wed, Jan 24, 2018 at 8:59 PM,  <[email protected]> wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> clr pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/whimsy.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
>      new fd861f1  Add error handling for discuss and vote pages
> fd861f1 is described below
>
> commit fd861f13ee85746ea7dcb67a4b7a338a7b63f238
> Author: Craig L Russell <[email protected]>
> AuthorDate: Wed Jan 24 17:59:24 2018 -0800
>
>     Add error handling for discuss and vote pages
> ---
>  www/project/icla/main.rb                   |  89 ++++++++-------
>  www/project/icla/views/app.html.rb         |   5 +-
>  www/project/icla/views/pages/discuss.js.rb | 120 ++++++++++++---------
>  www/project/icla/views/pages/vote.js.rb    | 167 
> ++++++++++++++++-------------
>  4 files changed, 206 insertions(+), 175 deletions(-)
>
> diff --git a/www/project/icla/main.rb b/www/project/icla/main.rb
> index 8236625..c51f400 100755
> --- a/www/project/icla/main.rb
> +++ b/www/project/icla/main.rb
> @@ -9,6 +9,7 @@ require 'wunderbar/vue'
>  require 'wunderbar/bootstrap/theme'
>  require 'ruby2js/filter/functions'
>  require 'ruby2js/filter/require'
> +require 'json'
>
>  disable :logging # suppress log of requests to stderr/error.log
>
> @@ -34,15 +35,45 @@ helpers do
>        'pmcmail' => mailList
>      }
>    end
> +  def loadProgress(token)
> +    if @token
> +      # read the file corresponging to the token
> +      # the file name is '/srv/<token>.json
> +      @filename = '/srv/icla/' + token + '.json'

Caller passes @token.  Method receives this as token.  Then @token is
checked, and if set, token is used to construct a filename.  Works,
but seems odd.

Also, there is no need to store filename as an instance variable.  Remove the @?

Finally, something serious: check to make sure that token contains
only hex characters?  In particular, disallow starting with a dot or
any use of forward or backward slashes.

- Sam Ruby

Reply via email to