[freeside-commits] branch FREESIDE_4_BRANCH updated. fe0e5a810960200b5352128139e6b040ea08eada
Mitch Jackson
mitch at freeside.biz
Fri Sep 7 12:13:12 PDT 2018
The branch, FREESIDE_4_BRANCH has been updated
via fe0e5a810960200b5352128139e6b040ea08eada (commit)
via 05a2dd488791bb2468d40db07d17db5cfaa673fd (commit)
via fb4fda982de4c62e88edfb4302cd52c10ec95d8e (commit)
via f257c3be34aed5f651a1928f4bc18864c0d533bc (commit)
via be0fbca00a9293aa7984dca5724fa885f41dbd5e (commit)
from c2b4bc64544ee18dae0093306b79d3d9798a02c1 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit fe0e5a810960200b5352128139e6b040ea08eada
Author: Mitch Jackson <mitch at freeside.biz>
Date: Tue Aug 28 14:05:17 2018 -0400
RT# 80869 Improve cust_payby.paydate validation
diff --git a/FS/FS/cust_payby.pm b/FS/FS/cust_payby.pm
index 6d7ece6ca..c497059fa 100644
--- a/FS/FS/cust_payby.pm
+++ b/FS/FS/cust_payby.pm
@@ -315,7 +315,6 @@ sub check {
#encrypted #|| $self->ut_textn('payinfo')
#encrypted #|| $self->ut_textn('paycvv')
# || $self->ut_textn('paymask') #XXX something
- || $self->ut_daten('paydate')
|| $self->ut_numbern('paystart_month')
|| $self->ut_numbern('paystart_year')
|| $self->ut_numbern('payissue')
@@ -546,6 +545,9 @@ sub check {
return $error if $error;
}
+ $error = $self->ut_daten('paydate');
+ return $error if $error;
+
$self->SUPER::check;
}
commit 05a2dd488791bb2468d40db07d17db5cfaa673fd
Author: Mitch Jackson <mitch at freeside.biz>
Date: Sun Aug 26 18:11:15 2018 -0400
RT# 80869 freeside_upgrade fix for bad payment expiration dates
diff --git a/FS/FS/cust_payby.pm b/FS/FS/cust_payby.pm
index 477700dc8..6d7ece6ca 100644
--- a/FS/FS/cust_payby.pm
+++ b/FS/FS/cust_payby.pm
@@ -921,9 +921,89 @@ sub _upgrade_data {
local $ignore_expired_card = 1;
local $ignore_invalid_card = 1;
$class->upgrade_set_cardtype;
+ $class->_upgrade_data_paydate_edgebug;
}
+=item _upgrade_data_paydate_edgebug
+
+Correct bad data injected into payment expire date column by Edge browser bug
+
+The month and year values may have an extra character injected into form POST
+data by Edge browser. It was possible for some bad month values to slip
+past data validation.
+
+If the stored value was out of range, it was causing payments screen to crash.
+We can detect and fix this by dropping the second digit.
+
+If the stored value is is 11 or 12, it's possible the user inputted a 1. In
+this case, the payment method will fail to authorize, but the record will
+not cause crashdumps for being out of range.
+
+In short, check for any expiration month > 12, and drop the extra digit
+
+=cut
+
+sub _upgrade_data_paydate_edgebug {
+ my $journal_label = 'cust_payby_paydate_edgebug';
+ return if FS::upgrade_journal->is_done( $journal_label );
+
+ my $oldAutoCommit = $FS::UID::AutoCommit;
+ local $FS::UID::AutoCommit = 0;
+
+ for my $row (
+ FS::Record::qsearch(
+ cust_payby => { paydate => { op => '!=', value => '' }}
+ )
+ ) {
+ next unless $row->ut_daten('paydate');
+
+ # paydate column stored in database has failed date validation
+ my $bad_paydate = $row->paydate;
+
+ my @date = split /[\-\/]/, $bad_paydate;
+ @date = @date[2,0,1] if $date[2] > 1900;
+
+ # Only autocorrecting when month > 12 - notify operator
+ unless ( $date[1] > 12 ) {
+ die sprintf(
+ 'Unable to correct bad paydate stored in cust_payby row '.
+ 'custpaybynum(%s) custnum(%s) paydate(%s)',
+ $row->custpaybynum,
+ $row->custnum,
+ $bad_paydate,
+ );
+ }
+
+ $date[1] = substr( $date[1], 0, 1 );
+ $row->paydate( join('-', @date ));
+
+ if ( my $error = $row->replace ) {
+ die sprintf(
+ 'Failed to autocorrect bad paydate stored in cust_payby row '.
+ 'custpaybynum(%s) custnum(%s) paydate(%s) - error: %s',
+ $row->custpaybynum,
+ $row->custnum,
+ $bad_paydate,
+ $error
+ );
+ }
+
+ warn sprintf(
+ 'Autocorrected bad paydate stored in cust_payby row '.
+ "custpaybynum(%s) custnum(%s) old-paydate(%s) new-paydate(%s)\n",
+ $row->custpaybynum,
+ $row->custnum,
+ $bad_paydate,
+ $row->paydate,
+ );
+
+ }
+
+ FS::upgrade_journal->set_done( $journal_label );
+ dbh->commit unless $oldAutoCommit;
+}
+
=head1 BUGS
=head1 SEE ALSO
commit fb4fda982de4c62e88edfb4302cd52c10ec95d8e
Author: Mitch Jackson <mitch at mitchjacksontech.com>
Date: Sun Aug 26 17:11:38 2018 -0400
RT# 80869 Improve cust_payby.paydate validation
diff --git a/FS/FS/Record.pm b/FS/FS/Record.pm
index 6ea936892..08d681e47 100644
--- a/FS/FS/Record.pm
+++ b/FS/FS/Record.pm
@@ -3207,6 +3207,60 @@ sub ut_enumn {
: '';
}
+=item ut_date COLUMN
+
+Check/untaint a column containing a date string.
+
+Date will be normalized to YYYY-MM-DD format
+
+=cut
+
+sub ut_date {
+ my ( $self, $field ) = @_;
+ my $value = $self->getfield( $field );
+
+ my @date = split /[\-\/]/, $value;
+ if ( scalar(@date) == 3 ) {
+ @date = @date[2,0,1] if $date[2] >= 1900;
+
+ local $@;
+ my $ymd;
+ eval {
+ # DateTime will die given invalid date
+ $ymd = DateTime->new(
+ year => $date[0],
+ month => $date[1],
+ day => $date[2],
+ )->ymd('-');
+ };
+
+ unless( $@ ) {
+ $self->setfield( $field, $ymd ) unless $value eq $ymd;
+ return '';
+ }
+
+ }
+ return "Illegal (date) field $field: $value";
+}
+
+=item ut_daten COLUMN
+
+Check/untaint a column containing a date string.
+
+Column may be null.
+
+Date will be normalized to YYYY-MM-DD format
+
+=cut
+
+sub ut_daten {
+ my ( $self, $field ) = @_;
+
+ $self->getfield( $field ) =~ /^()$/
+ ? $self->setfield( $field, '' )
+ : $self->ut_date( $field );
+}
+
=item ut_flag COLUMN
Check/untaint a column if it contains either an empty string or 'Y'. This
diff --git a/FS/FS/cust_payby.pm b/FS/FS/cust_payby.pm
index 704741f3d..477700dc8 100644
--- a/FS/FS/cust_payby.pm
+++ b/FS/FS/cust_payby.pm
@@ -315,7 +315,7 @@ sub check {
#encrypted #|| $self->ut_textn('payinfo')
#encrypted #|| $self->ut_textn('paycvv')
# || $self->ut_textn('paymask') #XXX something
- #later #|| $self->ut_textn('paydate')
+ || $self->ut_daten('paydate')
|| $self->ut_numbern('paystart_month')
|| $self->ut_numbern('paystart_year')
|| $self->ut_numbern('payissue')
commit f257c3be34aed5f651a1928f4bc18864c0d533bc
Author: Mitch Jackson <mitch at freeside.biz>
Date: Sun Aug 26 17:07:46 2018 -0400
RT# 80869 Harden process payment screen against Edge browser bug
diff --git a/httemplate/elements/city.html b/httemplate/elements/city.html
index 4e9a60940..05250fef5 100644
--- a/httemplate/elements/city.html
+++ b/httemplate/elements/city.html
@@ -132,14 +132,14 @@ function <% $pre %>county_changed(what, callback) {}
>
% unless ( $opt{'disable_empty'} ) {
- <OPTION VALUE="" <% $opt{city} eq '' ? 'SELECTED' : '' %>><% $opt{empty_label} %>
+ <OPTION VALUE="" <% $opt{city} eq '' ? 'SELECTED' : '' %>><% $opt{empty_label} %></OPTION>
% }
% foreach my $city ( @cities ) {
<OPTION VALUE="<% $city |h %>"
<% $city eq $opt{city} ? 'SELECTED' : '' %>
- ><% $city eq $opt{empty_data_value} ? $opt{empty_data_label} : $city %>
+ ><% $city eq $opt{empty_data_value} ? $opt{empty_data_label} : $city %></OPTION>
% }
diff --git a/httemplate/elements/cust_payby_new.html b/httemplate/elements/cust_payby_new.html
index 7ed049686..8b1d93d59 100644
--- a/httemplate/elements/cust_payby_new.html
+++ b/httemplate/elements/cust_payby_new.html
@@ -4,7 +4,6 @@
% my( $payinfo, $paycvv, $month, $year ) = ( '', '', '', '' );
% my $payname = $cust_main->first. ' '. $cust_main->getfield('last');
% my $location = $cust_main->bill_location;
-
<TR>
<TH ALIGN="right"><% mt('Card number') |h %></TH>
<TD COLSPAN=7>
@@ -15,21 +14,17 @@
<TH><% mt('Exp.') |h %></TH>
<TD>
<SELECT NAME="month">
-% for ( ( map "0$_", 1 .. 9 ), 10 .. 12 ) {
-
- <OPTION<% $_ == $month ? ' SELECTED' : '' %>><% $_ %>
+% for my $mm ( map{ sprintf( '%02d', $_ ) } (1..12) ) {
+ <OPTION value="<% $mm %>"<% $mm == $month ? ' SELECTED' : '' %>><% $mm %></OPTION>
% }
-
</SELECT>
</TD>
<TD> / </TD>
<TD>
<SELECT NAME="year">
-% my @a = localtime; for ( $a[5]+1900 .. $a[5]+1915 ) {
-
- <OPTION<% $_ == $year ? ' SELECTED' : '' %>><% $_ %>
+% my @a = localtime; for my $yyyy ( $a[5]+1900 .. $a[5]+1915 ) {
+ <OPTION value="<% $yyyy %>"<% $yyyy == $year ? ' SELECTED' : '' %>><% $yyyy %></OPTION>
% }
-
</SELECT>
</TD>
</TR>
@@ -162,7 +157,7 @@
<% mt('as') |h %>
<SELECT NAME="weight">
% for ( 1 .. 1+scalar(grep { $_->payby =~ /^(CARD|CHEK)$/ } @cust_payby) ) {
- <OPTION VALUE="<%$_%>"><% mt( $weight{$_} ) |h %>
+ <OPTION VALUE="<%$_%>"><% mt( $weight{$_} ) |h %></OPTION>
% }
</SELECT>
% } else {
diff --git a/httemplate/elements/select-country.html b/httemplate/elements/select-country.html
index c98147907..286826752 100644
--- a/httemplate/elements/select-country.html
+++ b/httemplate/elements/select-country.html
@@ -91,15 +91,13 @@ Example:
>
% unless ( $opt{'disable_empty'} ) {
- <OPTION VALUE=""><% $opt{'empty_label'} || '(all)' %>
+ <OPTION VALUE=""><% $opt{'empty_label'} || '(all)' %></OPTION>
% }
% foreach my $country ( @all_countries ) {
-
- <OPTION VALUE="<% $country |h %>"
- <% $country eq $opt{'country'} ? ' SELECTED' : '' %>
- ><% FS::geocode_Mixin->code2country($country). " ($country)" %>
-
+ <OPTION VALUE="<% $country |h %>"<% $country eq $opt{'country'} ? ' SELECTED' : '' %>>
+ <% FS::geocode_Mixin->code2country($country). " ($country)" |h %>
+ </OPTION>
% }
</SELECT>
diff --git a/httemplate/elements/select-month_year.html b/httemplate/elements/select-month_year.html
index 62c10b15f..406c13b21 100644
--- a/httemplate/elements/select-month_year.html
+++ b/httemplate/elements/select-month_year.html
@@ -3,16 +3,15 @@
<% $empty ? '<OPTION VALUE="">' : '' %>
% foreach ( 1 .. 12 ) {
- <OPTION<% $_ == $mon ? ' SELECTED' : '' %> VALUE="<% $_ %>"><% $mon[$_-1] %>
+ <OPTION<% $_ == $mon ? ' SELECTED' : '' %> VALUE="<% sprintf('%02d', $_) %>"><% $mon[$_-1] %></OPTION>
% }
-
</SELECT>/<SELECT NAME="<% $prefix %>_year" SIZE="1" <% $disabled%>>
<% $empty ? '<OPTION VALUE="">' : '' %>
% for ( $start_year .. $end_year ) {
- <OPTION<% $_ == $year ? ' SELECTED' : '' %> VALUE="<% $_ %>"><% $_ %>
+ <OPTION<% $_ == $year ? ' SELECTED' : '' %> VALUE="<% $_ %>"><% $_ %></OPTION>
% }
</SELECT>
diff --git a/httemplate/elements/select-state.html b/httemplate/elements/select-state.html
index 3fb559734..8db157b92 100644
--- a/httemplate/elements/select-state.html
+++ b/httemplate/elements/select-state.html
@@ -27,16 +27,13 @@ Example:
>
% unless ( $opt{'disable_empty'} ) {
- <OPTION VALUE=""<% $opt{state} eq '' ? ' SELECTED' : '' %>><% $opt{empty_label} %>
+ <OPTION VALUE=""<% $opt{state} eq '' ? ' SELECTED' : '' %>><% $opt{empty_label} %></OPTION>
% }
% foreach my $state ( keys %states ) {
-
- <OPTION VALUE="<% $state |h %>"<% $state eq $opt{'state'} ? ' SELECTED' : '' %>><% $states{$state} || '(n/a)' |h %>
-
+ <OPTION VALUE="<% $state |h %>"<% $state eq $opt{'state'} ? ' SELECTED' : '' %>><% $states{$state} || '(n/a)' |h %></OPTION>
% }
-
</SELECT>
<%init>
diff --git a/httemplate/elements/select-table.html b/httemplate/elements/select-table.html
index a52fdfaaa..d86b7ee43 100644
--- a/httemplate/elements/select-table.html
+++ b/httemplate/elements/select-table.html
@@ -83,11 +83,11 @@ Example:
% || ( $value eq $pre_opt );
<OPTION VALUE="<% $pre_opt %>"
<% $selected ? 'SELECTED' : '' %>
- ><% $pre_label %>
+ ><% $pre_label %></OPTION>
% }
% unless ( $opt{'multiple'} || $opt{'disable_empty'} ) {
- <OPTION VALUE=""><% $opt{'empty_label'} || 'all' %>
+ <OPTION VALUE=""><% $opt{'empty_label'} || 'all' %></OPTION>
% }
% foreach my $record (
@@ -118,7 +118,7 @@ Example:
? &{ $opt{'label_callback'} }( $record )
: $record->$name_col()
|h
- %>
+ %></OPTION>
% }
% while ( @post_options ) {
@@ -128,7 +128,7 @@ Example:
% || ( $value eq $post_opt );
<OPTION VALUE="<% $post_opt %>"
<% $selected ? 'SELECTED' : '' %>
- ><% $post_label %>
+ ><% $post_label %></OPTION>
% }
</SELECT>
diff --git a/httemplate/misc/process/payment.cgi b/httemplate/misc/process/payment.cgi
index d0e589661..7747bcbea 100644
--- a/httemplate/misc/process/payment.cgi
+++ b/httemplate/misc/process/payment.cgi
@@ -100,11 +100,11 @@ if ( (my $custpaybynum = scalar($cgi->param('custpaybynum'))) > 0 ) {
# use new info
##
- $cgi->param('year') =~ /^(\d+)$/
+ $cgi->param('year') =~ /^(\d{4})/
or errorpage("illegal year ". $cgi->param('year'));
$year = $1;
- $cgi->param('month') =~ /^(\d+)$/
+ $cgi->param('month') =~ /^(\d{2})/
or errorpage("illegal month ". $cgi->param('month'));
$month = $1;
commit be0fbca00a9293aa7984dca5724fa885f41dbd5e
Author: Mitch Jackson <mitch at freeside.biz>
Date: Fri Aug 24 22:31:14 2018 -0400
RT# 81183 Fix crash processing payment with new payment method
diff --git a/httemplate/misc/process/payment.cgi b/httemplate/misc/process/payment.cgi
index 939fc38c2..d0e589661 100644
--- a/httemplate/misc/process/payment.cgi
+++ b/httemplate/misc/process/payment.cgi
@@ -211,15 +211,21 @@ if ( (my $custpaybynum = scalar($cgi->param('custpaybynum'))) > 0 ) {
my $error = '';
my $paynum = '';
-my $paydate;
-if ($cust_payby->paydate) { $paydate = "$year-$month-01"; }
-else { $paydate = "2037-12-01"; }
if ( $cgi->param('batch') ) {
$error = 'Prepayment discounts not supported with batched payments'
if $discount_term;
+ # Invalid payment expire dates are replaced with 2037-12-01 (why?)
+ my $paydate = "${year}-${month}-01";
+ {
+ use DateTime;
+ local $@;
+ eval { DateTime->new({ year => $year, month => $month, day => 1 }) };
+ $paydate = '2037-12-01' if $@;
+ }
+
$error ||= $cust_main->batch_card(
'payby' => $payby,
'amount' => $amount,
-----------------------------------------------------------------------
Summary of changes:
FS/FS/Record.pm | 54 +++++++++++++++++++
FS/FS/cust_payby.pm | 84 +++++++++++++++++++++++++++++-
httemplate/elements/city.html | 4 +-
httemplate/elements/cust_payby_new.html | 15 ++----
httemplate/elements/select-country.html | 10 ++--
httemplate/elements/select-month_year.html | 5 +-
httemplate/elements/select-state.html | 7 +--
httemplate/elements/select-table.html | 8 +--
httemplate/misc/process/payment.cgi | 16 ++++--
9 files changed, 167 insertions(+), 36 deletions(-)
More information about the freeside-commits
mailing list