Skip to content

Commit 75a4bd1

Browse files
syoyosyoyo
committed
Add zero-value check when parsing `f' line. Fixes #140.
1 parent 7c7335c commit 75a4bd1

4 files changed

Lines changed: 115 additions & 36 deletions

File tree

‎models/issue-140-zero-face-idx.mtl‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
newmtl main
2+
Kd 1 1 1

‎models/issue-140-zero-face-idx.obj‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
mtllib issue-140-zero-face-idx.mtl
2+
3+
v -0.5 -0.5 0
4+
v 0.5 -0.5 0
5+
v 0.5 0.5 0
6+
v -0.5 0.5 0
7+
8+
vt 0 0 0
9+
vt 1 0 0
10+
vt 1 1 0
11+
vt 0 1 0
12+
13+
vn 0 0 -1
14+
15+
usemtl main
16+
f 0/0/0 1/1/0 3/3/0
17+
f 1/1/0 3/3/0 2/2/0

‎tests/tester.cc‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,23 @@ TEST_CASE("norm_texopts", "[norm]") {
643643

644644
}
645645

646+
TEST_CASE("zero-face-idx-value", "[Issue140]") {
647+
tinyobj::attrib_t attrib;
648+
std::vector<tinyobj::shape_t> shapes;
649+
std::vector<tinyobj::material_t> materials;
650+
651+
std::string err;
652+
bool ret = tinyobj::LoadObj(&attrib, &shapes, &materials, &err, "../models/issue-140-zero-face-idx.obj", gMtlBasePath);
653+
654+
655+
if (!err.empty()) {
656+
std::cerr << err << std::endl;
657+
}
658+
REQUIRE(false == ret);
659+
REQUIRE(!err.empty());
660+
661+
}
662+
646663
#if 0
647664
int
648665
main(

‎tiny_obj_loader.h‎

Lines changed: 79 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,11 @@ namespace tinyobj {
100100
// cube_left | cube_right
101101

102102
#ifdef TINYOBJLOADER_USE_DOUBLE
103-
//#pragma message "using double"
104-
typedef double real_t;
103+
//#pragma message "using double"
104+
typedef double real_t;
105105
#else
106-
//#pragma message "using float"
107-
typedef float real_t;
106+
//#pragma message "using float"
107+
typedef float real_t;
108108
#endif
109109

110110
typedef enum {
@@ -119,7 +119,7 @@ typedef enum {
119119
} texture_type_t;
120120

121121
typedef struct {
122-
texture_type_t type; // -type (default TEXTURE_TYPE_NONE)
122+
texture_type_t type; // -type (default TEXTURE_TYPE_NONE)
123123
real_t sharpness; // -boost (default 1.0?)
124124
real_t brightness; // base_value in -mm option (default 0)
125125
real_t contrast; // gain_value in -mm option (default 1)
@@ -364,7 +364,6 @@ namespace tinyobj {
364364

365365
MaterialReader::~MaterialReader() {}
366366

367-
368367
struct vertex_index {
369368
int v_idx, vt_idx, vn_idx;
370369
vertex_index() : v_idx(-1), vt_idx(-1), vn_idx(-1) {}
@@ -428,10 +427,27 @@ static std::istream &safeGetline(std::istream &is, std::string &t) {
428427
#define IS_NEW_LINE(x) (((x) == '\r') || ((x) == '\n') || ((x) == '\0'))
429428

430429
// Make index zero-base, and also support relative index.
431-
static inline int fixIndex(int idx, int n) {
432-
if (idx > 0) return idx - 1;
433-
if (idx == 0) return 0;
434-
return n + idx; // negative value = relative
430+
static inline bool fixIndex(int idx, int n, int *ret) {
431+
if (!ret) {
432+
return false;
433+
}
434+
435+
if (idx > 0) {
436+
(*ret) = idx - 1;
437+
return true;
438+
}
439+
440+
if (idx == 0) {
441+
// zero is not allowed according to the spec.
442+
return false;
443+
}
444+
445+
if (idx < 0) {
446+
(*ret) = n + idx; // negative value = relative
447+
return true;
448+
}
449+
450+
return false; // never reach here.
435451
}
436452

437453
static inline std::string parseString(const char **token) {
@@ -584,9 +600,9 @@ static bool tryParseDouble(const char *s, const char *s_end, double *result) {
584600
}
585601

586602
assemble:
587-
*result =
588-
(sign == '+' ? 1 : -1) *
589-
(exponent ? std::ldexp(mantissa * std::pow(5.0, exponent), exponent) : mantissa);
603+
*result = (sign == '+' ? 1 : -1) *
604+
(exponent ? std::ldexp(mantissa * std::pow(5.0, exponent), exponent)
605+
: mantissa);
590606
return true;
591607
fail:
592608
return false;
@@ -603,16 +619,16 @@ static inline real_t parseReal(const char **token, double default_value = 0.0) {
603619
}
604620

605621
static inline void parseReal2(real_t *x, real_t *y, const char **token,
606-
const double default_x = 0.0,
607-
const double default_y = 0.0) {
622+
const double default_x = 0.0,
623+
const double default_y = 0.0) {
608624
(*x) = parseReal(token, default_x);
609625
(*y) = parseReal(token, default_y);
610626
}
611627

612-
static inline void parseReal3(real_t *x, real_t *y, real_t *z, const char **token,
613-
const double default_x = 0.0,
614-
const double default_y = 0.0,
615-
const double default_z = 0.0) {
628+
static inline void parseReal3(real_t *x, real_t *y, real_t *z,
629+
const char **token, const double default_x = 0.0,
630+
const double default_y = 0.0,
631+
const double default_z = 0.0) {
616632
(*x) = parseReal(token, default_x);
617633
(*y) = parseReal(token, default_y);
618634
(*z) = parseReal(token, default_z);
@@ -694,37 +710,57 @@ static tag_sizes parseTagTriple(const char **token) {
694710
}
695711

696712
// Parse triples with index offsets: i, i/j/k, i//k, i/j
697-
static vertex_index parseTriple(const char **token, int vsize, int vnsize,
698-
int vtsize) {
713+
static bool parseTriple(const char **token, int vsize, int vnsize, int vtsize,
714+
vertex_index *ret) {
715+
if (!ret) {
716+
return false;
717+
}
718+
699719
vertex_index vi(-1);
700720

701-
vi.v_idx = fixIndex(atoi((*token)), vsize);
721+
if (!fixIndex(atoi((*token)), vsize, &(vi.v_idx))) {
722+
return false;
723+
}
724+
702725
(*token) += strcspn((*token), "/ \t\r");
703726
if ((*token)[0] != '/') {
704-
return vi;
727+
(*ret) = vi;
728+
return true;
705729
}
706730
(*token)++;
707731

708732
// i//k
709733
if ((*token)[0] == '/') {
710734
(*token)++;
711-
vi.vn_idx = fixIndex(atoi((*token)), vnsize);
735+
if (!fixIndex(atoi((*token)), vnsize, &(vi.vn_idx))) {
736+
return false;
737+
}
712738
(*token) += strcspn((*token), "/ \t\r");
713-
return vi;
739+
(*ret) = vi;
740+
return true;
714741
}
715742

716743
// i/j/k or i/j
717-
vi.vt_idx = fixIndex(atoi((*token)), vtsize);
744+
if (!fixIndex(atoi((*token)), vtsize, &(vi.vt_idx))) {
745+
return false;
746+
}
747+
718748
(*token) += strcspn((*token), "/ \t\r");
719749
if ((*token)[0] != '/') {
720-
return vi;
750+
(*ret) = vi;
751+
return true;
721752
}
722753

723754
// i/j/k
724755
(*token)++; // skip '/'
725-
vi.vn_idx = fixIndex(atoi((*token)), vnsize);
756+
if (!fixIndex(atoi((*token)), vnsize, &(vi.vn_idx))) {
757+
return false;
758+
}
726759
(*token) += strcspn((*token), "/ \t\r");
727-
return vi;
760+
761+
(*ret) = vi;
762+
763+
return true;
728764
}
729765

730766
// Parse raw triples: i, i/j/k, i//k, i/j
@@ -813,15 +849,15 @@ static bool ParseTextureNameAndOption(std::string *texname,
813849
} else if ((0 == strncmp(token, "-o", 2)) && IS_SPACE((token[2]))) {
814850
token += 3;
815851
parseReal3(&(texopt->origin_offset[0]), &(texopt->origin_offset[1]),
816-
&(texopt->origin_offset[2]), &token);
852+
&(texopt->origin_offset[2]), &token);
817853
} else if ((0 == strncmp(token, "-s", 2)) && IS_SPACE((token[2]))) {
818854
token += 3;
819855
parseReal3(&(texopt->scale[0]), &(texopt->scale[1]), &(texopt->scale[2]),
820-
&token, 1.0, 1.0, 1.0);
856+
&token, 1.0, 1.0, 1.0);
821857
} else if ((0 == strncmp(token, "-t", 2)) && IS_SPACE((token[2]))) {
822858
token += 3;
823859
parseReal3(&(texopt->turbulence[0]), &(texopt->turbulence[1]),
824-
&(texopt->turbulence[2]), &token);
860+
&(texopt->turbulence[2]), &token);
825861
} else if ((0 == strncmp(token, "-type", 5)) && IS_SPACE((token[5]))) {
826862
token += 5;
827863
texopt->type = parseTextureType((&token), TEXTURE_TYPE_NONE);
@@ -1532,9 +1568,16 @@ bool LoadObj(attrib_t *attrib, std::vector<shape_t> *shapes,
15321568
face.reserve(3);
15331569

15341570
while (!IS_NEW_LINE(token[0])) {
1535-
vertex_index vi = parseTriple(&token, static_cast<int>(v.size() / 3),
1536-
static_cast<int>(vn.size() / 3),
1537-
static_cast<int>(vt.size() / 2));
1571+
vertex_index vi;
1572+
if (!parseTriple(&token, static_cast<int>(v.size() / 3),
1573+
static_cast<int>(vn.size() / 3),
1574+
static_cast<int>(vt.size() / 2), &vi)) {
1575+
if (err) {
1576+
(*err) = "Failed parse `f' line(e.g. zero value for face index).\n";
1577+
}
1578+
return false;
1579+
}
1580+
15381581
face.push_back(vi);
15391582
size_t n = strspn(token, " \t\r");
15401583
token += n;
@@ -1622,7 +1665,7 @@ bool LoadObj(attrib_t *attrib, std::vector<shape_t> *shapes,
16221665
// flush previous face group.
16231666
bool ret = exportFaceGroupToShape(&shape, faceGroup, tags, material, name,
16241667
triangulate);
1625-
(void)ret; // return value not used.
1668+
(void)ret; // return value not used.
16261669

16271670
if (shape.mesh.indices.size() > 0) {
16281671
shapes->push_back(shape);

0 commit comments

Comments
 (0)