Skip to content

Commit a73708c

Browse files
xavierjeffersonxavierjefferson
committed
Safer conversion of Job Ids
1 parent a602062 commit a73708c

12 files changed

+122
-33
lines changed

‎Hangfire.FluentNHibernateStorage.Tests/FluentNHibernateStorageConnectionTests.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ public void CreateExpiredJob_CreatesAJobInTheStorage_AndSetsItsParameters()
169169
Assert.True(sqlJob.ExpireAt < createdAt.AddDays(1).AddMinutes(1));
170170

171171
var parameters = session.Query<_JobParameter>()
172-
.Where(i => i.Job.Id == int.Parse(jobId))
172+
.Where(i => i.Job.Id == long.Parse(jobId))
173173
.ToDictionary(x => x.Name, x => x.Value);
174174

175175
Assert.Equal("Value1", parameters["Key1"]);

‎Hangfire.FluentNHibernateStorage.Tests/FluentNHibernateWriteOnlyTransactionTests.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public static _Job InsertNewJob(SessionWrapper session, Action<_Job> action = nu
6565

6666
private static _Job GetTestJob(SessionWrapper connection, string jobId)
6767
{
68-
return connection.Query<_Job>().Single(i => i.Id == int.Parse(jobId));
68+
return connection.Query<_Job>().Single(i => i.Id == long.Parse(jobId));
6969
}
7070

7171
private static void UseSession(Action<SessionWrapper> action)

‎Hangfire.FluentNHibernateStorage.Tests/JobQueue/FluentNHibernateJobQueueMonitoringApiTests.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public void GetEnqueuedAndFetchedCount_ReturnsEqueuedCount_WhenExists()
4848
[CleanDatabase(IsolationLevel.ReadUncommitted)]
4949
public void GetEnqueuedJobIds_ReturnsCorrectResult()
5050
{
51-
int[] result = null; List<_Job> jobs = new List<_Job>();
51+
long[] result = null; List<_Job> jobs = new List<_Job>();
5252
_storage.UseSession(session =>
5353
{
5454

‎Hangfire.FluentNHibernateStorage.Tests/JobQueue/FluentNHibernateJobQueueTests.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ public void Dequeue_ShouldSetFetchedAt_OnlyForTheFetchedJob()
227227

228228
// Assert
229229
var otherJobFetchedAt = session.Query<_JobQueue>()
230-
.Where(i => i.Job.Id != int.Parse(payload.JobId))
230+
.Where(i => i.Job.Id != long.Parse(payload.JobId))
231231
.Select(i => i.FetchedAt)
232232
.Single();
233233

‎Hangfire.FluentNHibernateStorage/FluentNHibernateJobStorageConnection.cs

+48-9
Original file line numberDiff line numberDiff line change
@@ -95,18 +95,22 @@ public override void SetJobParameter(string id, string name, string value)
9595
{
9696
if (id == null) throw new ArgumentNullException("id");
9797
if (name == null) throw new ArgumentNullException("name");
98-
98+
var converter = JobIdConverter.Get(id);
99+
if (!converter.Valid)
100+
{
101+
return;
102+
}
99103
Storage.UseSession(session =>
100104
{
101105
var updated = session.CreateQuery(SQLHelper.UpdateJobParameterValueStatement)
102106
.SetParameter(SQLHelper.ValueParameterName, value)
103-
.SetParameter(SQLHelper.IdParameterName, int.Parse(id))
107+
.SetParameter(SQLHelper.IdParameterName, converter.Value)
104108
.ExecuteUpdate();
105109
if (updated == 0)
106110
{
107111
var jobParameter = new _JobParameter
108112
{
109-
Job = new _Job {Id = int.Parse(id)},
113+
Job = new _Job {Id = converter.Value},
110114
Name = name,
111115
Value = value
112116
};
@@ -120,25 +124,37 @@ public override void SetJobParameter(string id, string name, string value)
120124
public override string GetJobParameter(string id, string name)
121125
{
122126
if (id == null) throw new ArgumentNullException("id");
127+
var converter = JobIdConverter.Get(id);
128+
if (!converter.Valid)
129+
{
130+
return null;
131+
}
123132
if (name == null) throw new ArgumentNullException("name");
124133

125134
return Storage.UseSession(session =>
126135
session.Query<_JobParameter>()
127-
.Where(i => i.Job.Id == int.Parse(id) && i.Name == name)
136+
.Where(i => i.Job.Id == converter.Value && i.Name == name)
128137
.Select(i => i.Value)
129138
.SingleOrDefault());
130139
}
131140

132141
public override JobData GetJobData(string jobId)
133142
{
134143
if (jobId == null) throw new ArgumentNullException("jobId");
144+
var converter = JobIdConverter.Get(jobId);
135145

146+
if (!converter.Valid)
147+
{
148+
return null;
149+
}
150+
Logger.InfoFormat("Get job data for job '{0}'",jobId);
151+
136152
return Storage.UseSession(session =>
137153
{
138154
var jobData =
139155
session
140156
.Query<_Job>()
141-
.SingleOrDefault(i => i.Id == int.Parse(jobId));
157+
.SingleOrDefault(i => i.Id == converter.Value);
142158

143159
if (jobData == null) return null;
144160

@@ -170,11 +186,15 @@ public override JobData GetJobData(string jobId)
170186
public override StateData GetStateData(string jobId)
171187
{
172188
if (jobId == null) throw new ArgumentNullException("jobId");
173-
189+
var converter = JobIdConverter.Get(jobId);
190+
if (!converter.Valid)
191+
{
192+
return null;
193+
}
174194
return Storage.UseSession(session =>
175195
{
176196
var job = session.Query<_Job>()
177-
.Where(i => i.Id == int.Parse(jobId))
197+
.Where(i => i.Id == converter.Value)
178198
.Select(i => new {i.StateName, i.StateData, i.StateReason})
179199
.SingleOrDefault();
180200
if (job == null)
@@ -271,7 +291,7 @@ public override List<string> GetRangeFromSet(string key, int startingFrom, int e
271291
{
272292
return session.Query<_Set>()
273293
.OrderBy(i => i.Id)
274-
.Skip(startingFrom - 1)
294+
.Skip(startingFrom)
275295
.Take(endingAt - startingFrom + 1)
276296
.Select(i => i.Value)
277297
.ToList();
@@ -371,7 +391,7 @@ public override List<string> GetRangeFromList(string key, int startingFrom, int
371391
.OrderByDescending(i => i.Id)
372392
.Where(i => i.Key == key)
373393
.Select(i => i.Value)
374-
.Skip(startingFrom - 1)
394+
.Skip(startingFrom)
375395
.Take(endingAt - startingFrom + 1)
376396
.ToList();
377397
});
@@ -446,4 +466,23 @@ public override Dictionary<string, string> GetAllEntriesFromHash(string key)
446466
});
447467
}
448468
}
469+
470+
class JobIdConverter
471+
{
472+
public bool Valid { get; private set; }
473+
public long Value { get; private set; }
474+
475+
public static JobIdConverter Get(string jobId)
476+
{
477+
long tmp;
478+
var valid = long.TryParse(jobId, out tmp);
479+
return new JobIdConverter {Value = tmp, Valid = valid};
480+
}
481+
482+
private JobIdConverter()
483+
{
484+
485+
}
486+
487+
}
449488
}

‎Hangfire.FluentNHibernateStorage/FluentNHibernateWriteOnlyTransaction.cs

+24-8
Original file line numberDiff line numberDiff line change
@@ -56,38 +56,50 @@ private void DeleteByKeyValue<T>(string key, string value, SessionWrapper sessio
5656
public override void ExpireJob(string jobId, TimeSpan expireIn)
5757
{
5858
Logger.TraceFormat("ExpireJob jobId={0}", jobId);
59-
59+
var converter = JobIdConverter.Get(jobId);
60+
if (!converter.Valid)
61+
{
62+
return;
63+
}
6064
AcquireJobLock();
6165

6266
QueueCommand(session =>
6367
session.CreateQuery(SQLHelper.UpdateJobExpireAtStatement)
64-
.SetParameter(SQLHelper.IdParameterName, int.Parse(jobId))
68+
.SetParameter(SQLHelper.IdParameterName, converter.Value)
6569
.SetParameter(SQLHelper.ValueParameterName, session.Storage.UtcNow.Add(expireIn))
6670
.ExecuteUpdate());
6771
}
6872

6973
public override void PersistJob(string jobId)
7074
{
7175
Logger.TraceFormat("PersistJob jobId={0}", jobId);
72-
76+
var converter = JobIdConverter.Get(jobId);
77+
if (!converter.Valid)
78+
{
79+
return;
80+
}
7381
AcquireJobLock();
7482

7583
QueueCommand(session =>
7684
session.CreateQuery(SQLHelper.UpdateJobExpireAtStatement)
7785
.SetParameter(SQLHelper.ValueParameterName, null)
78-
.SetParameter(SQLHelper.IdParameterName, int.Parse(jobId))
86+
.SetParameter(SQLHelper.IdParameterName, converter.Value)
7987
.ExecuteUpdate());
8088
}
8189

8290
public override void SetJobState(string jobId, IState state)
8391
{
8492
Logger.TraceFormat("SetJobState jobId={0}", jobId);
85-
93+
var converter = JobIdConverter.Get(jobId);
94+
if (!converter.Valid)
95+
{
96+
return;
97+
}
8698
AcquireStateLock();
8799
AcquireJobLock();
88100
QueueCommand(session =>
89101
{
90-
var job = session.Query<_Job>().SingleOrDefault(i => i.Id == int.Parse(jobId));
102+
var job = session.Query<_Job>().SingleOrDefault(i => i.Id == converter.Value);
91103
if (job != null)
92104
{
93105
var sqlState = new _JobState
@@ -115,13 +127,17 @@ public override void SetJobState(string jobId, IState state)
115127
public override void AddJobState(string jobId, IState state)
116128
{
117129
Logger.TraceFormat("AddJobState jobId={0}, state={1}", jobId, state);
118-
130+
var converter = JobIdConverter.Get(jobId);
131+
if (!converter.Valid)
132+
{
133+
return;
134+
}
119135
AcquireStateLock();
120136
QueueCommand(session =>
121137
{
122138
session.Insert(new _JobState
123139
{
124-
Job = new _Job {Id = int.Parse(jobId)},
140+
Job = new _Job {Id = converter.Value},
125141
Name = state.Name,
126142
Reason = state.Reason,
127143
CreatedAt = session.Storage.UtcNow,

‎Hangfire.FluentNHibernateStorage/Hangfire.FluentNHibernateStorage.csproj

+1
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
<Compile Include="Entities\_DistributedLock.cs" />
6969
<Compile Include="Entities\_Dual.cs" />
7070
<Compile Include="FluentNHibernateStorageFactory.cs" />
71+
<Compile Include="JobIdConverter.cs" />
7172
<Compile Include="Maps\Constants.cs" />
7273
<Compile Include="Maps\IntIdMap.cs" />
7374
<Compile Include="Maps\ObjectHelper.cs" />

‎Hangfire.FluentNHibernateStorage/JobIdConverter.cs

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
namespace Hangfire.FluentNHibernateStorage
2+
{
3+
class JobIdConverter
4+
{
5+
public bool Valid { get; private set; }
6+
public long Value { get; private set; }
7+
8+
public static JobIdConverter Get(string jobId)
9+
{
10+
long tmp;
11+
var valid = long.TryParse(jobId, out tmp);
12+
return new JobIdConverter {Value = tmp, Valid = valid};
13+
}
14+
15+
private JobIdConverter()
16+
{
17+
18+
}
19+
20+
}
21+
}

‎Hangfire.FluentNHibernateStorage/JobQueue/FluentNHibernateJobQueue.cs

+6-1
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,14 @@ public IFetchedJob Dequeue(string[] queues, CancellationToken cancellationToken)
101101

102102
public void Enqueue(SessionWrapper session, string queue, string jobId)
103103
{
104+
var converter = JobIdConverter.Get(jobId);
105+
if (!converter.Valid)
106+
{
107+
return;
108+
}
104109
session.Insert(new _JobQueue
105110
{
106-
Job = session.Query<_Job>().SingleOrDefault(i => i.Id == int.Parse(jobId)),
111+
Job = session.Query<_Job>().SingleOrDefault(i => i.Id == converter.Value),
107112
Queue = queue
108113
});
109114
session.Flush();

‎Hangfire.FluentNHibernateStorage/JobQueue/FluentNHibernateJobQueueMonitoringApi.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public IEnumerable<long> GetEnqueuedJobIds(string queue, int from, int perPage)
4444
.OrderBy(i => i.Id)
4545
.Where(i => i.Queue == queue)
4646
.Select(i => i.Job.Id)
47-
.Skip(from - 1)
47+
.Skip(from)
4848
.Take(perPage)
4949
.ToList();
5050
});
@@ -57,7 +57,7 @@ public IEnumerable<long> GetFetchedJobIds(string queue, int from, int perPage)
5757
return _storage.UseSession(session =>
5858
{
5959
return session.Query<_JobQueue>().Where(i => (i.FetchedAt != null) & (i.Queue == queue))
60-
.OrderBy(i => i.Id).Skip(from - 1).Take(perPage).Select(i => i.Id).ToList();
60+
.OrderBy(i => i.Id).Skip(from).Take(perPage).Select(i => i.Id).ToList();
6161
});
6262
}
6363

‎Hangfire.FluentNHibernateStorage/Monitoring/FluentNHibernateMonitoringApi.cs

+8-2
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,14 @@ public IList<ServerDto> Servers()
7777

7878
public JobDetailsDto JobDetails(string jobId)
7979
{
80+
var converter = JobIdConverter.Get(jobId);
81+
if (!converter.Valid)
82+
{
83+
return null;
84+
}
8085
return UseStatefulTransaction(session =>
8186
{
82-
var job = session.Query<_Job>().SingleOrDefault(i => i.Id == int.Parse(jobId));
87+
var job = session.Query<_Job>().SingleOrDefault(i => i.Id == converter.Value);
8388
if (job == null) return null;
8489

8590
var parameters = job.Parameters.ToDictionary(x => x.Name, x => x.Value);
@@ -137,6 +142,7 @@ long CountStats(string key)
137142

138143
return new StatisticsDto
139144
{
145+
140146
Enqueued = GetJobStatusCount("Enqueued"),
141147
Failed = GetJobStatusCount("Failed"),
142148
Processing = GetJobStatusCount("Processing"),
@@ -354,7 +360,7 @@ private JobList<TDto> GetJobs<TDto>(
354360
var a = session.Query<_Job>()
355361
.OrderBy(i => i.Id)
356362
.Where(i => i.StateName == stateName)
357-
.Skip(from - 1)
363+
.Skip(from)
358364
.Take(count)
359365
.ToList();
360366

‎nuspecs/Hangfire.FluentNHibernateStorage.nuspec

+8-7
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<package>
44
<metadata>
55
<id>Hangfire.FluentNHibernateStorage</id>
6-
<version>1.1.2040</version>
6+
<version>1.1.2041</version>
77
<authors>Xavier Jefferson</authors>
88
<owners>XavierJ</owners>
99
<licenseUrl>https://www.gnu.org/licenses/lgpl-3.0.en.html</licenseUrl>
@@ -12,12 +12,13 @@
1212
<requireLicenseAcceptance>false</requireLicenseAcceptance>
1313
<description>A Hangfire storage provider for SQL Server, MySQL, Oracle, DB/2, Postgres, and Firebird</description>
1414
<releaseNotes>
15-
* Fix bug in GetTTL method
16-
* Switch all applicable ID column types from int to long
17-
* Fix index offset issue in methods that look for a range of table entries
18-
* Properly implement function to get fetched job ids
19-
* Return correct exception during NHibernate session factory generation
20-
* Optimize where the schema update is occurring during session factory generation
15+
* Safer conversion of Job Ids
16+
* Fix bug in GetTTL method
17+
* Switch all applicable ID column types from int to long
18+
* Fix index offset issue in methods that look for a range of table entries
19+
* Properly implement function to get fetched job ids
20+
* Return correct exception during NHibernate session factory generation
21+
* Optimize where the schema update is occurring during session factory generation
2122
</releaseNotes>
2223
<copyright>Copyright © 2017 Xavier Jefferson</copyright>
2324
<tags>hangfire storage provider mssql mysql oracle postgresql firebird db2</tags>

0 commit comments

Comments
 (0)